| Message ID | 20230813122344.14142-1-paskripkin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | crypto: fix uninit-value in af_alg_free_resources | expand |
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
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
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;
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(+)