* [PATCH] af_key: add __GFP_ZERO flag for compose_sadb_supported in function pfkey_register
@ 2022-03-04 8:33 Haimin Zhang
2022-03-04 11:19 ` Steffen Klassert
0 siblings, 1 reply; 2+ messages in thread
From: Haimin Zhang @ 2022-03-04 8:33 UTC (permalink / raw)
To: Steffen Klassert, Herbert Xu, David S. Miller, Jakub Kicinski,
netdev
Cc: Haimin Zhang, TCS Robot
From: Haimin Zhang <tcs_kernel@tencent.com>
Add __GFP_ZERO flag for compose_sadb_supported in function pfkey_register
to initialize the buffer of supp_skb.
Reported-by: TCS Robot <tcs_robot@tencent.com>
Signed-off-by: Haimin Zhang <tcs_kernel@tencent.com>
---
This can cause a kernel-info-leak problem.
1. Function pfkey_register calls compose_sadb_supported to request a sk_buff.
2. compose_sadb_supported calls alloc_sbk to allocate a sk_buff, but it doesn't zero it.
3. If auth_len is greater 0, then compose_sadb_supported treats the memory as a struct sadb_supported and begins to initialize.
But it just initializes the field sadb_supported_len and field sadb_supported_exttype without field sadb_supported_reserved.
```
slab_post_alloc_hook build/../mm/slab.h:737 [inline]
slab_alloc_node build/../mm/slub.c:3247 [inline]
__kmalloc_node_track_caller+0x8da/0x11d0 build/../mm/slub.c:4975
kmalloc_reserve build/../net/core/skbuff.c:354 [inline]
__alloc_skb+0x545/0xf90 build/../net/core/skbuff.c:426
alloc_skb build/../include/linux/skbuff.h:1158 [inline]
compose_sadb_supported build/../net/key/af_key.c:1631 [inline]
pfkey_register+0x3c6/0xdb0 build/../net/key/af_key.c:1702
pfkey_process build/../net/key/af_key.c:2837 [inline]
pfkey_sendmsg+0x16bb/0x1c60 build/../net/key/af_key.c:3676
sock_sendmsg_nosec build/../net/socket.c:705 [inline]
sock_sendmsg build/../net/socket.c:725 [inline]
____sys_sendmsg+0xe11/0x12c0 build/../net/socket.c:2413
___sys_sendmsg+0x4a7/0x530 build/../net/socket.c:2467
__sys_sendmsg build/../net/socket.c:2496 [inline]
__do_sys_sendmsg build/../net/socket.c:2505 [inline]
__se_sys_sendmsg build/../net/socket.c:2503 [inline]
__x64_sys_sendmsg+0x3ef/0x570 build/../net/socket.c:2503
do_syscall_x64 build/../arch/x86/entry/common.c:51 [inline]
do_syscall_64+0x54/0xd0 build/../arch/x86/entry/common.c:82
entry_SYSCALL_64_after_hwframe+0x44/0xae
```
4. When this message was received, pfkey_recvmsg calls skb_copy_datagram_msg to copy the data to a userspace buffer.
```
instrument_copy_to_user build/../include/linux/instrumented.h:121 [inline]
copyout build/../lib/iov_iter.c:154 [inline]
_copy_to_iter+0x65d/0x2510 build/../lib/iov_iter.c:668
copy_to_iter build/../include/linux/uio.h:162 [inline]
simple_copy_to_iter+0xf3/0x140 build/../net/core/datagram.c:519
__skb_datagram_iter+0x2d5/0x11b0 build/../net/core/datagram.c:425
skb_copy_datagram_iter+0xdc/0x270 build/../net/core/datagram.c:533
skb_copy_datagram_msg build/../include/linux/skbuff.h:3696 [inline]
pfkey_recvmsg+0x43e/0xb50 build/../net/key/af_key.c:3710
____sys_recvmsg+0x590/0xb00
___sys_recvmsg+0x37a/0xb70 build/../net/socket.c:2674
do_recvmmsg+0x6b3/0x11a0 build/../net/socket.c:2768
__sys_recvmmsg build/../net/socket.c:2847 [inline]
__do_sys_recvmmsg build/../net/socket.c:2870 [inline]
__se_sys_recvmmsg build/../net/socket.c:2863 [inline]
__x64_sys_recvmmsg+0x2af/0x500 build/../net/socket.c:2863
do_syscall_x64 build/../arch/x86/entry/common.c:51 [inline]
do_syscall_64+0x54/0xd0 build/../arch/x86/entry/common.c:82
entry_SYSCALL_64_after_hwframe+0x44/0xae
```
5. The following is debug information:
Bytes 20-23 of 176 are uninitialized
Memory access of size 176 starts at ffff88815e686000
Data copied to user address 0000000020000300
net/key/af_key.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/key/af_key.c b/net/key/af_key.c
index de24a7d474df..cf5433a2e31a 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -1699,7 +1699,7 @@ static int pfkey_register(struct sock *sk, struct sk_buff *skb, const struct sad
xfrm_probe_algs();
- supp_skb = compose_sadb_supported(hdr, GFP_KERNEL);
+ supp_skb = compose_sadb_supported(hdr, GFP_KERNEL | __GFP_ZERO);
if (!supp_skb) {
if (hdr->sadb_msg_satype != SADB_SATYPE_UNSPEC)
pfk->registered &= ~(1<<hdr->sadb_msg_satype);
--
2.32.0 (Apple Git-132)
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] af_key: add __GFP_ZERO flag for compose_sadb_supported in function pfkey_register
2022-03-04 8:33 [PATCH] af_key: add __GFP_ZERO flag for compose_sadb_supported in function pfkey_register Haimin Zhang
@ 2022-03-04 11:19 ` Steffen Klassert
0 siblings, 0 replies; 2+ messages in thread
From: Steffen Klassert @ 2022-03-04 11:19 UTC (permalink / raw)
To: Haimin Zhang
Cc: Herbert Xu, David S. Miller, Jakub Kicinski, netdev, Haimin Zhang,
TCS Robot
On Fri, Mar 04, 2022 at 04:33:27PM +0800, Haimin Zhang wrote:
> From: Haimin Zhang <tcs_kernel@tencent.com>
>
> Add __GFP_ZERO flag for compose_sadb_supported in function pfkey_register
> to initialize the buffer of supp_skb.
>
> Reported-by: TCS Robot <tcs_robot@tencent.com>
> Signed-off-by: Haimin Zhang <tcs_kernel@tencent.com>
Can you please add at least a bit of the below problem description
to the commit message? Also please add a 'Fixes:' tag so that
it can be backtpored to the stable trees.
Thanks!
> ---
> This can cause a kernel-info-leak problem.
> 1. Function pfkey_register calls compose_sadb_supported to request a sk_buff.
> 2. compose_sadb_supported calls alloc_sbk to allocate a sk_buff, but it doesn't zero it.
> 3. If auth_len is greater 0, then compose_sadb_supported treats the memory as a struct sadb_supported and begins to initialize.
> But it just initializes the field sadb_supported_len and field sadb_supported_exttype without field sadb_supported_reserved.
> ```
> slab_post_alloc_hook build/../mm/slab.h:737 [inline]
> slab_alloc_node build/../mm/slub.c:3247 [inline]
> __kmalloc_node_track_caller+0x8da/0x11d0 build/../mm/slub.c:4975
> kmalloc_reserve build/../net/core/skbuff.c:354 [inline]
> __alloc_skb+0x545/0xf90 build/../net/core/skbuff.c:426
> alloc_skb build/../include/linux/skbuff.h:1158 [inline]
> compose_sadb_supported build/../net/key/af_key.c:1631 [inline]
> pfkey_register+0x3c6/0xdb0 build/../net/key/af_key.c:1702
> pfkey_process build/../net/key/af_key.c:2837 [inline]
> pfkey_sendmsg+0x16bb/0x1c60 build/../net/key/af_key.c:3676
> sock_sendmsg_nosec build/../net/socket.c:705 [inline]
> sock_sendmsg build/../net/socket.c:725 [inline]
> ____sys_sendmsg+0xe11/0x12c0 build/../net/socket.c:2413
> ___sys_sendmsg+0x4a7/0x530 build/../net/socket.c:2467
> __sys_sendmsg build/../net/socket.c:2496 [inline]
> __do_sys_sendmsg build/../net/socket.c:2505 [inline]
> __se_sys_sendmsg build/../net/socket.c:2503 [inline]
> __x64_sys_sendmsg+0x3ef/0x570 build/../net/socket.c:2503
> do_syscall_x64 build/../arch/x86/entry/common.c:51 [inline]
> do_syscall_64+0x54/0xd0 build/../arch/x86/entry/common.c:82
> entry_SYSCALL_64_after_hwframe+0x44/0xae
> ```
>
> 4. When this message was received, pfkey_recvmsg calls skb_copy_datagram_msg to copy the data to a userspace buffer.
> ```
> instrument_copy_to_user build/../include/linux/instrumented.h:121 [inline]
> copyout build/../lib/iov_iter.c:154 [inline]
> _copy_to_iter+0x65d/0x2510 build/../lib/iov_iter.c:668
> copy_to_iter build/../include/linux/uio.h:162 [inline]
> simple_copy_to_iter+0xf3/0x140 build/../net/core/datagram.c:519
> __skb_datagram_iter+0x2d5/0x11b0 build/../net/core/datagram.c:425
> skb_copy_datagram_iter+0xdc/0x270 build/../net/core/datagram.c:533
> skb_copy_datagram_msg build/../include/linux/skbuff.h:3696 [inline]
> pfkey_recvmsg+0x43e/0xb50 build/../net/key/af_key.c:3710
> ____sys_recvmsg+0x590/0xb00
> ___sys_recvmsg+0x37a/0xb70 build/../net/socket.c:2674
> do_recvmmsg+0x6b3/0x11a0 build/../net/socket.c:2768
> __sys_recvmmsg build/../net/socket.c:2847 [inline]
> __do_sys_recvmmsg build/../net/socket.c:2870 [inline]
> __se_sys_recvmmsg build/../net/socket.c:2863 [inline]
> __x64_sys_recvmmsg+0x2af/0x500 build/../net/socket.c:2863
> do_syscall_x64 build/../arch/x86/entry/common.c:51 [inline]
> do_syscall_64+0x54/0xd0 build/../arch/x86/entry/common.c:82
> entry_SYSCALL_64_after_hwframe+0x44/0xae
> ```
>
> 5. The following is debug information:
> Bytes 20-23 of 176 are uninitialized
> Memory access of size 176 starts at ffff88815e686000
> Data copied to user address 0000000020000300
>
> net/key/af_key.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/key/af_key.c b/net/key/af_key.c
> index de24a7d474df..cf5433a2e31a 100644
> --- a/net/key/af_key.c
> +++ b/net/key/af_key.c
> @@ -1699,7 +1699,7 @@ static int pfkey_register(struct sock *sk, struct sk_buff *skb, const struct sad
>
> xfrm_probe_algs();
>
> - supp_skb = compose_sadb_supported(hdr, GFP_KERNEL);
> + supp_skb = compose_sadb_supported(hdr, GFP_KERNEL | __GFP_ZERO);
> if (!supp_skb) {
> if (hdr->sadb_msg_satype != SADB_SATYPE_UNSPEC)
> pfk->registered &= ~(1<<hdr->sadb_msg_satype);
> --
> 2.32.0 (Apple Git-132)
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-03-04 11:19 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-03-04 8:33 [PATCH] af_key: add __GFP_ZERO flag for compose_sadb_supported in function pfkey_register Haimin Zhang
2022-03-04 11:19 ` Steffen Klassert
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).