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