1. Patchew
  2. linux
  3. View series

[PATCH net] xfrm: fix one more kernel-infoleak in algo dumping

Petr Vaganov posted 1 patch 1 month, 3 weeks ago
There is a newer version of this series
net/xfrm/xfrm_user.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[PATCH net] xfrm: fix one more kernel-infoleak in algo dumping
Posted by Petr Vaganov 1 month, 3 weeks ago 
During fuzz testing, the following issue was discovered:

BUG: KMSAN: kernel-infoleak in _copy_to_iter+0x598/0x2a30
 _copy_to_iter+0x598/0x2a30
 __skb_datagram_iter+0x168/0x1060
 skb_copy_datagram_iter+0x5b/0x220
 netlink_recvmsg+0x362/0x1700
 sock_recvmsg+0x2dc/0x390
 __sys_recvfrom+0x381/0x6d0
 __x64_sys_recvfrom+0x130/0x200
 x64_sys_call+0x32c8/0x3cc0
 do_syscall_64+0xd8/0x1c0
 entry_SYSCALL_64_after_hwframe+0x79/0x81

Uninit was stored to memory at:
 copy_to_user_state_extra+0xcc1/0x1e00
 dump_one_state+0x28c/0x5f0
 xfrm_state_walk+0x548/0x11e0
 xfrm_dump_sa+0x1e0/0x840
 netlink_dump+0x943/0x1c40
 __netlink_dump_start+0x746/0xdb0
 xfrm_user_rcv_msg+0x429/0xc00
 netlink_rcv_skb+0x613/0x780
 xfrm_netlink_rcv+0x77/0xc0
 netlink_unicast+0xe90/0x1280
 netlink_sendmsg+0x126d/0x1490
 __sock_sendmsg+0x332/0x3d0
 ____sys_sendmsg+0x863/0xc30
 ___sys_sendmsg+0x285/0x3e0
 __x64_sys_sendmsg+0x2d6/0x560
 x64_sys_call+0x1316/0x3cc0
 do_syscall_64+0xd8/0x1c0
 entry_SYSCALL_64_after_hwframe+0x79/0x81

Uninit was created at:
 __kmalloc+0x571/0xd30
 attach_auth+0x106/0x3e0
 xfrm_add_sa+0x2aa0/0x4230
 xfrm_user_rcv_msg+0x832/0xc00
 netlink_rcv_skb+0x613/0x780
 xfrm_netlink_rcv+0x77/0xc0
 netlink_unicast+0xe90/0x1280
 netlink_sendmsg+0x126d/0x1490
 __sock_sendmsg+0x332/0x3d0
 ____sys_sendmsg+0x863/0xc30
 ___sys_sendmsg+0x285/0x3e0
 __x64_sys_sendmsg+0x2d6/0x560
 x64_sys_call+0x1316/0x3cc0
 do_syscall_64+0xd8/0x1c0
 entry_SYSCALL_64_after_hwframe+0x79/0x81

Bytes 328-379 of 732 are uninitialized
Memory access of size 732 starts at ffff88800e18e000
Data copied to user address 00007ff30f48aff0

CPU: 2 PID: 18167 Comm: syz-executor.0 Not tainted 6.8.11 #1
Hardware name: 
QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014

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)));
+	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
Re: [PATCH net] xfrm: fix one more kernel-infoleak in algo dumping
Posted by Simon Horman 1 month, 3 weeks ago 
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
> 
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.