<meta http-equiv="Content-Type" content="text/html; charset=GB18030"><font face="ثخجه" size="2"><div> Hi,</div><div><br></div><div> I found a "copy-before-check" issue in ow_ipsec_sa_common_param_fill()</div><div> (drivers/common/cnxk/cnxk_security.c). When AES-GMAC is used as the</div><div> authentication algorithm through the CN20K (OW family) inline IPsec</div><div> path, the auth key is copied into cipher_key[32] before the AES key</div><div> length validation switch is reached. If the key length exceeds 32,</div><div> the memcpy overflows into the adjacent SA member, and only afterward</div><div> is the invalid length detected and rejected with -EINVAL.</div><div><br></div><div> This is the OW-family counterpart of the same pattern in</div><div> ot_ipsec_sa_common_param_fill() (OT/CN10K family). Both share</div><div> identical code structure, but they are independent functions</div><div> operating on different SA structure types.</div><div><br></div><div> Confirmed against current HEAD (8dc80afd, 2026-03-05).</div><div><br></div><div> == Affected Code Flow ==</div><div><br></div><div> ow_ipsec_sa_common_param_fill() in cnxk_security.c, lines 1210-1392.</div><div><br></div><div> Step 1 -- AES-GMAC sets key from auth xform (lines 1334-1341):</div><div><br></div><div> case RTE_CRYPTO_AUTH_AES_GMAC:</div><div> w2->s.auth_type = ROC_IE_SA_AUTH_AES_GMAC;</div><div> key = auth_xfrm->auth.key.data;</div><div> length = auth_xfrm->auth.key.length; // e.g., 40</div><div> memcpy(salt_key, &ipsec_xfrm->salt, 4);</div><div> tmp_salt = (uint32_t *)salt_key;</div><div> *tmp_salt = rte_be_to_cpu_32(*tmp_salt);</div><div> break;</div><div><br></div><div> Step 2 -- Key is copied unconditionally (lines 1366-1368):</div><div><br></div><div> if (key != NULL && length != 0) {</div><div> /* Copy encryption key */</div><div> memcpy(cipher_key, key, length); // OVERFLOW: 40 > 32</div><div> ...</div><div> }</div><div><br></div><div> Step 3 -- AES key length validated AFTER the copy (lines 1374-1392):</div><div><br></div><div> if (w2->s.enc_type == ROC_IE_SA_ENC_AES_CBC ||</div><div> w2->s.enc_type == ROC_IE_SA_ENC_AES_CCM ||</div><div> w2->s.enc_type == ROC_IE_SA_ENC_AES_CTR ||</div><div> w2->s.enc_type == ROC_IE_SA_ENC_AES_GCM ||</div><div> w2->s.enc_type == ROC_IE_SA_ENC_AES_CCM ||</div><div> w2->s.auth_type == ROC_IE_SA_AUTH_AES_GMAC) {</div><div> switch (length) {</div><div> case ROC_CPT_AES128_KEY_LEN: ...</div><div> case ROC_CPT_AES192_KEY_LEN: ...</div><div> case ROC_CPT_AES256_KEY_LEN: ...</div><div> default:</div><div> plt_err("Invalid AES key length");</div><div> return -EINVAL; // rejected, but memcpy already done</div><div> }</div><div> }</div><div><br></div><div> For AES-GMAC, auth_type == ROC_IE_SA_AUTH_AES_GMAC matches the</div><div> condition at line 1377, so the validation IS entered. A length of</div><div> 40 hits default and returns -EINVAL. But the memcpy at line 1368</div><div> has already written 40 bytes into cipher_key[32].</div><div><br></div><div> == Affected Structure Layout ==</div><div><br></div><div> When called from cnxk_ow_ipsec_inb_sa_fill() (line 1509), the</div><div> cipher_key parameter points to sa->cipher_key:</div><div><br></div><div> struct roc_ow_ipsec_inb_sa { (roc_ie_ow.h:288)</div><div> ...</div><div> /* Word4 - Word7 */</div><div> uint8_t cipher_key[ROC_CTX_MAX_CKEY_LEN]; /* 32 bytes */</div><div><br></div><div> /* Word8 - Word9 */</div><div> union {</div><div> struct {</div><div> uint32_t rsvd8;</div><div> uint8_t salt[4]; /* active member */</div><div> } s;</div><div> uint64_t u64;</div><div> } w8;</div><div> uint64_t rsvd9;</div><div> ...</div><div> };</div><div><br></div><div> Static assertions confirm the layout (roc_ie_ow.h:406-407):</div><div><br></div><div> PLT_STATIC_ASSERT(offsetof(struct roc_ow_ipsec_inb_sa,</div><div> cipher_key) == 4 * sizeof(uint64_t)); /* offset 32 */</div><div> PLT_STATIC_ASSERT(offsetof(struct roc_ow_ipsec_inb_sa,</div><div> w8) == 8 * sizeof(uint64_t)); /* offset 64 */</div><div><br></div><div> A 40-byte key writes 32 bytes into cipher_key, then 8 bytes into</div><div> w8, overwriting salt[4].</div><div><br></div><div> For outbound (cnxk_ow_ipsec_outb_sa_fill, line 1619), cipher_key[32]</div><div> is followed by union roc_ow_ipsec_outb_iv iv (roc_ie_ow.h:495).</div><div><br></div><div> == Why This Is Low-Severity ==</div><div><br></div><div> Unlike the DES/3DES bugs ([014]-[016]) where the function returns</div><div> success and the corrupted SA is used by hardware, here:</div><div><br></div><div> 1. The switch at line 1378 catches invalid lengths and returns</div><div> -EINVAL.</div><div><br></div><div> 2. Callers properly propagate the error:</div><div> - cnxk_ow_ipsec_inb_sa_fill() checks rc at line 1511.</div><div> - cn20k_eth_sec_session_create() checks rc at line 794 and</div><div> jumps to the error path. The SA is not written to hardware.</div><div> - cn20k_eth_sec_session_update() checks at line 1066.</div><div><br></div><div> 3. The staging buffer (dev->inb.sa_dptr) is memset to zero before</div><div> every session creation (line 790), so the transient overflow</div><div> is cleared on next use.</div><div><br></div><div> The corrupted SA is never committed to hardware. The practical</div><div> impact is a transient write to a staging buffer that will be</div><div> zeroed before next use.</div><div><br></div><div> == All Affected Call Chains ==</div><div><br></div><div> cn20k_eth_sec_session_create() [cn20k_ethdev_sec.c:686]</div><div> inbound: cnxk_ow_ipsec_inb_sa_fill() [line 793]</div><div> outbound: cnxk_ow_ipsec_outb_sa_fill() [line 874]</div><div> -> ow_ipsec_sa_common_param_fill() [line 1509 / 1619]</div><div> -> memcpy(cipher_key, key, length) [line 1368]</div><div> -> switch(length) ... return -EINVAL [line 1378-1391]</div><div><br></div><div> cn20k_eth_sec_session_update() [cn20k_ethdev_sec.c:1030]</div><div> inbound: cnxk_ow_ipsec_inb_sa_fill() [line 1065]</div><div> outbound: cnxk_ow_ipsec_outb_sa_fill() [line 1095]</div><div> -> same path</div><div><br></div><div> == Relationship to Other cnxk Key Length Bugs ==</div><div><br></div><div> This is one of a family of related issues in the cnxk inline IPsec</div><div> paths, all caused by the inline ethdev drivers not calling</div><div> cnxk_ipsec_xform_verify() before SA fill:</div><div><br></div><div> [014]-[016]: DES/3DES cipher key has NO validation at all.</div><div> Function returns success. SA is used. Higher severity.</div><div> [017]: AES-XCBC auth key has NO validation in inbound path.</div><div> Function returns success. SA is used. Higher severity.</div><div> [018]: AES-GMAC key in OT helper: copy-before-check.</div><div> Function returns -EINVAL. SA not used. Lower severity.</div><div> [019]: AES-GMAC key in OW helper: copy-before-check.</div><div> Function returns -EINVAL. SA not used. Lower severity.</div><div> (this report)</div><div><br></div><div> All six can be fixed together by moving key length validation before</div><div> the memcpy in each helper, plus adding the missing DES/3DES and</div><div> AES-XCBC checks.</div><div><br></div><div> == Suggested Fix ==</div><div><br></div><div> Move the AES key length switch before the memcpy in</div><div> ow_ipsec_sa_common_param_fill(), and add DES/3DES validation:</div><div><br></div><div> if (key != NULL && length != 0) {</div><div> + /* Validate key length before copy */</div><div> + if (w2->s.enc_type == ROC_IE_SA_ENC_AES_CBC ||</div><div> + w2->s.enc_type == ROC_IE_SA_ENC_AES_CCM ||</div><div> + w2->s.enc_type == ROC_IE_SA_ENC_AES_CTR ||</div><div> + w2->s.enc_type == ROC_IE_SA_ENC_AES_GCM ||</div><div> + w2->s.auth_type == ROC_IE_SA_AUTH_AES_GMAC) {</div><div> + switch (length) {</div><div> + case 16: case 24: case 32: break;</div><div> + default: return -EINVAL;</div><div> + }</div><div> + }</div><div> + if (w2->s.enc_type == ROC_IE_SA_ENC_DES_CBC && length != 8)</div><div> + return -EINVAL;</div><div> + if (w2->s.enc_type == ROC_IE_SA_ENC_3DES_CBC && length != 24)</div><div> + return -EINVAL;</div><div> +</div><div> memcpy(cipher_key, key, length);</div><div> ...</div><div> }</div><div><br></div><div> - /* Remove the duplicate AES key length block that was here */</div><div><br></div><div> Apply the same restructuring to ot_ipsec_sa_common_param_fill()</div><div> and on_fill_ipsec_common_sa() / on_ipsec_sa_ctl_set().</div><div><br></div><div><div>Best regards,</div><div>Pengpeng Hou</div><div><a href="mailto:pengpeng@iscas.ac.cn" rel="noopener" target="_blank" style="outline: none; cursor: pointer; color: rgb(30, 84, 148);">pengpeng@iscas.ac.cn</a></div></div><div><br></div></font>