* [PATCH] crypto: fix uninit-value in af_alg_free_resources
@ 2023-08-13 12:23 Pavel Skripkin
2023-08-14 8:26 ` David Howells
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Pavel Skripkin @ 2023-08-13 12:23 UTC (permalink / raw)
To: herbert, davem, dhowells, pabeni
Cc: linux-crypto, linux-kernel, Pavel Skripkin, syzbot+cba21d50095623218389
Syzbot was able to trigger use of uninitialized memory in
af_alg_free_resources.
Bug is caused by missing initialization of rsgl->sgl.need_unpin before
adding to rsgl_list. Then in case of extract_iter_to_sg() failure, rsgl
is left with uninitialized need_unpin which is read during clean up
BUG: KMSAN: uninit-value in af_alg_free_sg crypto/af_alg.c:545 [inline]
BUG: KMSAN: uninit-value in af_alg_free_areq_sgls crypto/af_alg.c:778 [inline]
BUG: KMSAN: uninit-value in af_alg_free_resources+0x3d1/0xf60 crypto/af_alg.c:1117
af_alg_free_sg crypto/af_alg.c:545 [inline]
af_alg_free_areq_sgls crypto/af_alg.c:778 [inline]
af_alg_free_resources+0x3d1/0xf60 crypto/af_alg.c:1117
_skcipher_recvmsg crypto/algif_skcipher.c:144 [inline]
...
Uninit was created at:
slab_post_alloc_hook+0x12f/0xb70 mm/slab.h:767
slab_alloc_node mm/slub.c:3470 [inline]
__kmem_cache_alloc_node+0x536/0x8d0 mm/slub.c:3509
__do_kmalloc_node mm/slab_common.c:984 [inline]
__kmalloc+0x121/0x3c0 mm/slab_common.c:998
kmalloc include/linux/slab.h:586 [inline]
sock_kmalloc+0x128/0x1c0 net/core/sock.c:2683
af_alg_alloc_areq+0x41/0x2a0 crypto/af_alg.c:1188
_skcipher_recvmsg crypto/algif_skcipher.c:71 [inline]
Fixes: c1abe6f570af ("crypto: af_alg: Use extract_iter_to_sg() to create scatterlists")
Reported-and-tested-by: syzbot+cba21d50095623218389@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=cba21d50095623218389
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
crypto/af_alg.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 06b15b9f661c..4e4f2d837481 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -1241,6 +1241,8 @@ int af_alg_get_rsgl(struct sock *sk, struct msghdr *msg, int flags,
return -ENOMEM;
}
+ rsgl->sgl.need_unpin = 0;
+
rsgl->sgl.sgt.sgl = rsgl->sgl.sgl;
rsgl->sgl.sgt.nents = 0;
rsgl->sgl.sgt.orig_nents = 0;
--
2.40.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: fix uninit-value in af_alg_free_resources
2023-08-13 12:23 [PATCH] crypto: fix uninit-value in af_alg_free_resources Pavel Skripkin
@ 2023-08-14 8:26 ` David Howells
2023-08-14 18:04 ` Pavel Skripkin
2023-08-14 18:03 ` [PATCH v2] " Pavel Skripkin
2023-08-18 13:21 ` David Howells
2 siblings, 1 reply; 6+ messages in thread
From: David Howells @ 2023-08-14 8:26 UTC (permalink / raw)
To: Pavel Skripkin
Cc: dhowells, herbert, davem, pabeni, linux-crypto, linux-kernel,
syzbot+cba21d50095623218389
Pavel Skripkin <paskripkin@gmail.com> wrote:
> Syzbot was able to trigger use of uninitialized memory in
> af_alg_free_resources.
>
> Bug is caused by missing initialization of rsgl->sgl.need_unpin before
> adding to rsgl_list. Then in case of extract_iter_to_sg() failure, rsgl
> is left with uninitialized need_unpin which is read during clean up
Looks feasible :-).
> + rsgl->sgl.need_unpin = 0;
> +
The blank line isn't really necessary and it's a bool, so can you use 'false'
rather than '0'?
Alternatively, it might be better to move:
rsgl->sgl.need_unpin =
iov_iter_extract_will_pin(&msg->msg_iter);
up instead.
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] crypto: fix uninit-value in af_alg_free_resources
2023-08-13 12:23 [PATCH] crypto: fix uninit-value in af_alg_free_resources Pavel Skripkin
2023-08-14 8:26 ` David Howells
@ 2023-08-14 18:03 ` Pavel Skripkin
2023-08-18 10:33 ` Herbert Xu
2023-08-18 13:21 ` David Howells
2 siblings, 1 reply; 6+ messages in thread
From: Pavel Skripkin @ 2023-08-14 18:03 UTC (permalink / raw)
To: herbert, davem, dhowells, pabeni
Cc: linux-crypto, linux-kernel, Pavel Skripkin, syzbot+cba21d50095623218389
Syzbot was able to trigger use of uninitialized memory in
af_alg_free_resources.
Bug is caused by missing initialization of rsgl->sgl.need_unpin before
adding to rsgl_list. Then in case of extract_iter_to_sg() failure, rsgl
is left with uninitialized need_unpin which is read during clean up
BUG: KMSAN: uninit-value in af_alg_free_sg crypto/af_alg.c:545 [inline]
BUG: KMSAN: uninit-value in af_alg_free_areq_sgls crypto/af_alg.c:778 [inline]
BUG: KMSAN: uninit-value in af_alg_free_resources+0x3d1/0xf60 crypto/af_alg.c:1117
af_alg_free_sg crypto/af_alg.c:545 [inline]
af_alg_free_areq_sgls crypto/af_alg.c:778 [inline]
af_alg_free_resources+0x3d1/0xf60 crypto/af_alg.c:1117
_skcipher_recvmsg crypto/algif_skcipher.c:144 [inline]
...
Uninit was created at:
slab_post_alloc_hook+0x12f/0xb70 mm/slab.h:767
slab_alloc_node mm/slub.c:3470 [inline]
__kmem_cache_alloc_node+0x536/0x8d0 mm/slub.c:3509
__do_kmalloc_node mm/slab_common.c:984 [inline]
__kmalloc+0x121/0x3c0 mm/slab_common.c:998
kmalloc include/linux/slab.h:586 [inline]
sock_kmalloc+0x128/0x1c0 net/core/sock.c:2683
af_alg_alloc_areq+0x41/0x2a0 crypto/af_alg.c:1188
_skcipher_recvmsg crypto/algif_skcipher.c:71 [inline]
Fixes: c1abe6f570af ("crypto: af_alg: Use extract_iter_to_sg() to create scatterlists")
Reported-and-tested-by: syzbot+cba21d50095623218389@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=cba21d50095623218389
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
Changes since v1:
- Move sgl.need_unpin initialization upper instead of
pre-initializing it with false as suggested by David
---
crypto/af_alg.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 06b15b9f661c..10efb56d8b48 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -1241,6 +1241,8 @@ int af_alg_get_rsgl(struct sock *sk, struct msghdr *msg, int flags,
return -ENOMEM;
}
+ rsgl->sgl.need_unpin =
+ iov_iter_extract_will_pin(&msg->msg_iter);
rsgl->sgl.sgt.sgl = rsgl->sgl.sgl;
rsgl->sgl.sgt.nents = 0;
rsgl->sgl.sgt.orig_nents = 0;
@@ -1255,8 +1257,6 @@ int af_alg_get_rsgl(struct sock *sk, struct msghdr *msg, int flags,
}
sg_mark_end(rsgl->sgl.sgt.sgl + rsgl->sgl.sgt.nents - 1);
- rsgl->sgl.need_unpin =
- iov_iter_extract_will_pin(&msg->msg_iter);
/* chain the new scatterlist with previous one */
if (areq->last_rsgl)
--
2.40.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: fix uninit-value in af_alg_free_resources
2023-08-14 8:26 ` David Howells
@ 2023-08-14 18:04 ` Pavel Skripkin
0 siblings, 0 replies; 6+ messages in thread
From: Pavel Skripkin @ 2023-08-14 18:04 UTC (permalink / raw)
To: David Howells
Cc: herbert, davem, pabeni, linux-crypto, linux-kernel,
syzbot+cba21d50095623218389
Hi David,
David Howells <dhowells@redhat.com> says:
> Pavel Skripkin <paskripkin@gmail.com> wrote:
>
>> Syzbot was able to trigger use of uninitialized memory in
>> af_alg_free_resources.
>>
>> Bug is caused by missing initialization of rsgl->sgl.need_unpin before
>> adding to rsgl_list. Then in case of extract_iter_to_sg() failure, rsgl
>> is left with uninitialized need_unpin which is read during clean up
>
> Looks feasible :-).
>
>> + rsgl->sgl.need_unpin = 0;
>> +
>
> The blank line isn't really necessary and it's a bool, so can you use 'false'
> rather than '0'?
>
> Alternatively, it might be better to move:
>
> rsgl->sgl.need_unpin =
> iov_iter_extract_will_pin(&msg->msg_iter);
>
> up instead.
>
Thank you for review! I've just posted v2 :)
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] crypto: fix uninit-value in af_alg_free_resources
2023-08-14 18:03 ` [PATCH v2] " Pavel Skripkin
@ 2023-08-18 10:33 ` Herbert Xu
0 siblings, 0 replies; 6+ messages in thread
From: Herbert Xu @ 2023-08-18 10:33 UTC (permalink / raw)
To: Pavel Skripkin
Cc: davem, dhowells, pabeni, linux-crypto, linux-kernel,
syzbot+cba21d50095623218389
On Mon, Aug 14, 2023 at 09:03:41PM +0300, Pavel Skripkin wrote:
> Syzbot was able to trigger use of uninitialized memory in
> af_alg_free_resources.
>
> Bug is caused by missing initialization of rsgl->sgl.need_unpin before
> adding to rsgl_list. Then in case of extract_iter_to_sg() failure, rsgl
> is left with uninitialized need_unpin which is read during clean up
>
> BUG: KMSAN: uninit-value in af_alg_free_sg crypto/af_alg.c:545 [inline]
> BUG: KMSAN: uninit-value in af_alg_free_areq_sgls crypto/af_alg.c:778 [inline]
> BUG: KMSAN: uninit-value in af_alg_free_resources+0x3d1/0xf60 crypto/af_alg.c:1117
> af_alg_free_sg crypto/af_alg.c:545 [inline]
> af_alg_free_areq_sgls crypto/af_alg.c:778 [inline]
> af_alg_free_resources+0x3d1/0xf60 crypto/af_alg.c:1117
> _skcipher_recvmsg crypto/algif_skcipher.c:144 [inline]
> ...
>
> Uninit was created at:
> slab_post_alloc_hook+0x12f/0xb70 mm/slab.h:767
> slab_alloc_node mm/slub.c:3470 [inline]
> __kmem_cache_alloc_node+0x536/0x8d0 mm/slub.c:3509
> __do_kmalloc_node mm/slab_common.c:984 [inline]
> __kmalloc+0x121/0x3c0 mm/slab_common.c:998
> kmalloc include/linux/slab.h:586 [inline]
> sock_kmalloc+0x128/0x1c0 net/core/sock.c:2683
> af_alg_alloc_areq+0x41/0x2a0 crypto/af_alg.c:1188
> _skcipher_recvmsg crypto/algif_skcipher.c:71 [inline]
>
> Fixes: c1abe6f570af ("crypto: af_alg: Use extract_iter_to_sg() to create scatterlists")
> Reported-and-tested-by: syzbot+cba21d50095623218389@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=cba21d50095623218389
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
>
> Changes since v1:
> - Move sgl.need_unpin initialization upper instead of
> pre-initializing it with false as suggested by David
>
> ---
> crypto/af_alg.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] crypto: fix uninit-value in af_alg_free_resources
2023-08-13 12:23 [PATCH] crypto: fix uninit-value in af_alg_free_resources Pavel Skripkin
2023-08-14 8:26 ` David Howells
2023-08-14 18:03 ` [PATCH v2] " Pavel Skripkin
@ 2023-08-18 13:21 ` David Howells
2 siblings, 0 replies; 6+ messages in thread
From: David Howells @ 2023-08-18 13:21 UTC (permalink / raw)
To: Pavel Skripkin
Cc: dhowells, herbert, davem, pabeni, linux-crypto, linux-kernel,
syzbot+cba21d50095623218389
Pavel Skripkin <paskripkin@gmail.com> wrote:
> Syzbot was able to trigger use of uninitialized memory in
> af_alg_free_resources.
>
> Bug is caused by missing initialization of rsgl->sgl.need_unpin before
> adding to rsgl_list. Then in case of extract_iter_to_sg() failure, rsgl
> is left with uninitialized need_unpin which is read during clean up
>
> BUG: KMSAN: uninit-value in af_alg_free_sg crypto/af_alg.c:545 [inline]
> BUG: KMSAN: uninit-value in af_alg_free_areq_sgls crypto/af_alg.c:778 [inline]
> BUG: KMSAN: uninit-value in af_alg_free_resources+0x3d1/0xf60 crypto/af_alg.c:1117
> af_alg_free_sg crypto/af_alg.c:545 [inline]
> af_alg_free_areq_sgls crypto/af_alg.c:778 [inline]
> af_alg_free_resources+0x3d1/0xf60 crypto/af_alg.c:1117
> _skcipher_recvmsg crypto/algif_skcipher.c:144 [inline]
> ...
>
> Uninit was created at:
> slab_post_alloc_hook+0x12f/0xb70 mm/slab.h:767
> slab_alloc_node mm/slub.c:3470 [inline]
> __kmem_cache_alloc_node+0x536/0x8d0 mm/slub.c:3509
> __do_kmalloc_node mm/slab_common.c:984 [inline]
> __kmalloc+0x121/0x3c0 mm/slab_common.c:998
> kmalloc include/linux/slab.h:586 [inline]
> sock_kmalloc+0x128/0x1c0 net/core/sock.c:2683
> af_alg_alloc_areq+0x41/0x2a0 crypto/af_alg.c:1188
> _skcipher_recvmsg crypto/algif_skcipher.c:71 [inline]
>
> Fixes: c1abe6f570af ("crypto: af_alg: Use extract_iter_to_sg() to create scatterlists")
> Reported-and-tested-by: syzbot+cba21d50095623218389@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=cba21d50095623218389
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Reviewed-by: David Howells <dhowells@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-18 13:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-13 12:23 [PATCH] crypto: fix uninit-value in af_alg_free_resources Pavel Skripkin
2023-08-14 8:26 ` David Howells
2023-08-14 18:04 ` Pavel Skripkin
2023-08-14 18:03 ` [PATCH v2] " Pavel Skripkin
2023-08-18 10:33 ` Herbert Xu
2023-08-18 13:21 ` David Howells
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).