On Wed, Oct 02, 2024 at 11:17:24AM +0500, Petr Vaganov wrote:
During fuzz testing, the following issue was discovered:
BUG: KMSAN: kernel-infoleak in _copy_to_iter+0x598/0x2a30
...
Fixes copying of xfrm algorithms where some random data of the structure fields can end up in userspace. Padding in structures may be filled with random (possibly sensitve) data and should never be given directly to user-space.
A similar issue was resolved in the commit 8222d5910dae ("xfrm: Zero padding when dumping algos and encap")
Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
Fixes: c7a5899eb26e ("xfrm: redact SA secret with lockdown confidentiality") Cc: stable@vger.kernel.org Co-developed-by: Boris Tonofa b.tonofa@ideco.ru Signed-off-by: Boris Tonofa b.tonofa@ideco.ru Signed-off-by: Petr Vaganov p.vaganov@ideco.ru
net/xfrm/xfrm_user.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c index 55f039ec3d59..97faeb3574ea 100644 --- a/net/xfrm/xfrm_user.c +++ b/net/xfrm/xfrm_user.c @@ -1098,7 +1098,9 @@ static int copy_to_user_auth(struct xfrm_algo_auth *auth, struct sk_buff *skb) if (!nla) return -EMSGSIZE; ap = nla_data(nla);
- memcpy(ap, auth, sizeof(struct xfrm_algo_auth));
- strscpy_pad(ap->alg_name, auth->alg_name, sizeof(sizeof(ap->alg_name)));
Hi Petr and Boris,
The nested sizeof doesn't look right to me. I expect the length of the destination is simply sizeof(ap->alg_name).
And given that ap->alg_name is an array (which is why using sizeof is correct here), I believe the two-argument variant of strscpy_pad() can be used:
strscpy_pad(ap->alg_name, auth->alg_name);
As an aside, and not for this patch, there is a usage of strncpy() just above this hunk which looks like it could be converted to the two-argument variant of strscpy() or strncpy_pad() if it ought to be zero-padded.
- ap->alg_key_len = auth->alg_key_len;
- ap->alg_trunc_len = auth->alg_trunc_len; if (redact_secret && auth->alg_key_len) memset(ap->alg_key, 0, (auth->alg_key_len + 7) / 8); else
-- 2.46.1