| Message ID | 20210304152125.1052825-1-paskripkin@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] net: mac802154: Fix general protection fault | expand |
Hello. On 04.03.21 16:21, Pavel Skripkin wrote: > syzbot found general protection fault in crypto_destroy_tfm()[1]. > It was caused by wrong clean up loop in llsec_key_alloc(). > If one of the tfm array members is in IS_ERR() range it will > cause general protection fault in clean up function [1]. > > Call Trace: > crypto_free_aead include/crypto/aead.h:191 [inline] [1] > llsec_key_alloc net/mac802154/llsec.c:156 [inline] > mac802154_llsec_key_add+0x9e0/0xcc0 net/mac802154/llsec.c:249 > ieee802154_add_llsec_key+0x56/0x80 net/mac802154/cfg.c:338 > rdev_add_llsec_key net/ieee802154/rdev-ops.h:260 [inline] > nl802154_add_llsec_key+0x3d3/0x560 net/ieee802154/nl802154.c:1584 > genl_family_rcv_msg_doit+0x228/0x320 net/netlink/genetlink.c:739 > genl_family_rcv_msg net/netlink/genetlink.c:783 [inline] > genl_rcv_msg+0x328/0x580 net/netlink/genetlink.c:800 > netlink_rcv_skb+0x153/0x420 net/netlink/af_netlink.c:2502 > genl_rcv+0x24/0x40 net/netlink/genetlink.c:811 > netlink_unicast_kernel net/netlink/af_netlink.c:1312 [inline] > netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1338 > netlink_sendmsg+0x856/0xd90 net/netlink/af_netlink.c:1927 > sock_sendmsg_nosec net/socket.c:654 [inline] > sock_sendmsg+0xcf/0x120 net/socket.c:674 > ____sys_sendmsg+0x6e8/0x810 net/socket.c:2350 > ___sys_sendmsg+0xf3/0x170 net/socket.c:2404 > __sys_sendmsg+0xe5/0x1b0 net/socket.c:2433 > do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 > entry_SYSCALL_64_after_hwframe+0x44/0xae > > Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> > Reported-by: syzbot+9ec037722d2603a9f52e@syzkaller.appspotmail.com > Change-Id: I29f7ac641a039096d63d1e6070bb32cb5a3beb07 > --- > net/mac802154/llsec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/mac802154/llsec.c b/net/mac802154/llsec.c > index 585d33144c33..55550ead2ced 100644 > --- a/net/mac802154/llsec.c > +++ b/net/mac802154/llsec.c > @@ -152,7 +152,7 @@ llsec_key_alloc(const struct ieee802154_llsec_key *template) > crypto_free_sync_skcipher(key->tfm0); > err_tfm: > for (i = 0; i < ARRAY_SIZE(key->tfm); i++) > - if (key->tfm[i]) > + if (!IS_ERR_OR_NULL(key->tfm[i])) > crypto_free_aead(key->tfm[i]); > > kfree_sensitive(key); > Alex, are you happy with this patch now? I would like to get it applied. Waiting for your review or ack given you had comments on the first version. regards Stefan Schmidt
Hi, On Thu, 4 Mar 2021 at 10:25, Pavel Skripkin <paskripkin@gmail.com> wrote: > > syzbot found general protection fault in crypto_destroy_tfm()[1]. > It was caused by wrong clean up loop in llsec_key_alloc(). > If one of the tfm array members is in IS_ERR() range it will > cause general protection fault in clean up function [1]. > > Call Trace: > crypto_free_aead include/crypto/aead.h:191 [inline] [1] > llsec_key_alloc net/mac802154/llsec.c:156 [inline] > mac802154_llsec_key_add+0x9e0/0xcc0 net/mac802154/llsec.c:249 > ieee802154_add_llsec_key+0x56/0x80 net/mac802154/cfg.c:338 > rdev_add_llsec_key net/ieee802154/rdev-ops.h:260 [inline] > nl802154_add_llsec_key+0x3d3/0x560 net/ieee802154/nl802154.c:1584 > genl_family_rcv_msg_doit+0x228/0x320 net/netlink/genetlink.c:739 > genl_family_rcv_msg net/netlink/genetlink.c:783 [inline] > genl_rcv_msg+0x328/0x580 net/netlink/genetlink.c:800 > netlink_rcv_skb+0x153/0x420 net/netlink/af_netlink.c:2502 > genl_rcv+0x24/0x40 net/netlink/genetlink.c:811 > netlink_unicast_kernel net/netlink/af_netlink.c:1312 [inline] > netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1338 > netlink_sendmsg+0x856/0xd90 net/netlink/af_netlink.c:1927 > sock_sendmsg_nosec net/socket.c:654 [inline] > sock_sendmsg+0xcf/0x120 net/socket.c:674 > ____sys_sendmsg+0x6e8/0x810 net/socket.c:2350 > ___sys_sendmsg+0xf3/0x170 net/socket.c:2404 > __sys_sendmsg+0xe5/0x1b0 net/socket.c:2433 > do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 > entry_SYSCALL_64_after_hwframe+0x44/0xae > > Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> > Reported-by: syzbot+9ec037722d2603a9f52e@syzkaller.appspotmail.com > Change-Id: I29f7ac641a039096d63d1e6070bb32cb5a3beb07 I am sorry, I don't know the tag "Change-Id", I was doing a whole grep on Documentation/ without any luck. Dumb question: What is the meaning of it? - Alex
Hi! On Sun, 2021-04-04 at 20:43 -0400, Alexander Aring wrote: > Hi, > > On Thu, 4 Mar 2021 at 10:25, Pavel Skripkin <paskripkin@gmail.com> > wrote: > > > > syzbot found general protection fault in crypto_destroy_tfm()[1]. > > It was caused by wrong clean up loop in llsec_key_alloc(). > > If one of the tfm array members is in IS_ERR() range it will > > cause general protection fault in clean up function [1]. > > > > Call Trace: > > crypto_free_aead include/crypto/aead.h:191 [inline] [1] > > llsec_key_alloc net/mac802154/llsec.c:156 [inline] > > mac802154_llsec_key_add+0x9e0/0xcc0 net/mac802154/llsec.c:249 > > ieee802154_add_llsec_key+0x56/0x80 net/mac802154/cfg.c:338 > > rdev_add_llsec_key net/ieee802154/rdev-ops.h:260 [inline] > > nl802154_add_llsec_key+0x3d3/0x560 net/ieee802154/nl802154.c:1584 > > genl_family_rcv_msg_doit+0x228/0x320 net/netlink/genetlink.c:739 > > genl_family_rcv_msg net/netlink/genetlink.c:783 [inline] > > genl_rcv_msg+0x328/0x580 net/netlink/genetlink.c:800 > > netlink_rcv_skb+0x153/0x420 net/netlink/af_netlink.c:2502 > > genl_rcv+0x24/0x40 net/netlink/genetlink.c:811 > > netlink_unicast_kernel net/netlink/af_netlink.c:1312 [inline] > > netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1338 > > netlink_sendmsg+0x856/0xd90 net/netlink/af_netlink.c:1927 > > sock_sendmsg_nosec net/socket.c:654 [inline] > > sock_sendmsg+0xcf/0x120 net/socket.c:674 > > ____sys_sendmsg+0x6e8/0x810 net/socket.c:2350 > > ___sys_sendmsg+0xf3/0x170 net/socket.c:2404 > > __sys_sendmsg+0xe5/0x1b0 net/socket.c:2433 > > do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 > > entry_SYSCALL_64_after_hwframe+0x44/0xae > > > > Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> > > Reported-by: syzbot+9ec037722d2603a9f52e@syzkaller.appspotmail.com > > Change-Id: I29f7ac641a039096d63d1e6070bb32cb5a3beb07 > > I am sorry, I don't know the tag "Change-Id", I was doing a whole > grep > on Documentation/ without any luck. > I forgot to check the patch with ./scripts/checkpatch.pl :( > Dumb question: What is the meaning of it? This is for gerrit code review. This is required to push changes to gerrit public mirror. I'm using it to check patches with syzbot. Change ids are useless outside gerrit, so it shouldn't be here. Btw, should I sent v2 or this is already fixed? > > - Alex With regards, Pavel Skripkin
Hi, On Mon, 5 Apr 2021 at 01:45, Pavel Skripkin <paskripkin@gmail.com> wrote: > > Hi! > ... > > > > I forgot to check the patch with ./scripts/checkpatch.pl :( > > > Dumb question: What is the meaning of it? > > This is for gerrit code review. This is required to push changes to > gerrit public mirror. I'm using it to check patches with syzbot. Change > ids are useless outside gerrit, so it shouldn't be here. > > Btw, should I sent v2 or this is already fixed? Otherwise the patch looks good. May Stefan can fix this. Acked-by: Alexander Aring <aahringo@redhat.com> - Alex
Hello. On 05.04.21 13:50, Alexander Aring wrote: > Hi, > > On Mon, 5 Apr 2021 at 01:45, Pavel Skripkin <paskripkin@gmail.com> wrote: >> >> Hi! >> > ... >>> >> >> I forgot to check the patch with ./scripts/checkpatch.pl :( >> >>> Dumb question: What is the meaning of it? >> >> This is for gerrit code review. This is required to push changes to >> gerrit public mirror. I'm using it to check patches with syzbot. Change >> ids are useless outside gerrit, so it shouldn't be here. >> >> Btw, should I sent v2 or this is already fixed? > > Otherwise the patch looks good. May Stefan can fix this. > > Acked-by: Alexander Aring <aahringo@redhat.com> I removed the Change-ID locally here. This patch has been applied to the wpan tree and will be part of the next pull request to net. Thanks! regards Stefan Schmidt
diff --git a/net/mac802154/llsec.c b/net/mac802154/llsec.c index 585d33144c33..55550ead2ced 100644 --- a/net/mac802154/llsec.c +++ b/net/mac802154/llsec.c @@ -152,7 +152,7 @@ llsec_key_alloc(const struct ieee802154_llsec_key *template) crypto_free_sync_skcipher(key->tfm0); err_tfm: for (i = 0; i < ARRAY_SIZE(key->tfm); i++) - if (key->tfm[i]) + if (!IS_ERR_OR_NULL(key->tfm[i])) crypto_free_aead(key->tfm[i]); kfree_sensitive(key);
syzbot found general protection fault in crypto_destroy_tfm()[1]. It was caused by wrong clean up loop in llsec_key_alloc(). If one of the tfm array members is in IS_ERR() range it will cause general protection fault in clean up function [1]. Call Trace: crypto_free_aead include/crypto/aead.h:191 [inline] [1] llsec_key_alloc net/mac802154/llsec.c:156 [inline] mac802154_llsec_key_add+0x9e0/0xcc0 net/mac802154/llsec.c:249 ieee802154_add_llsec_key+0x56/0x80 net/mac802154/cfg.c:338 rdev_add_llsec_key net/ieee802154/rdev-ops.h:260 [inline] nl802154_add_llsec_key+0x3d3/0x560 net/ieee802154/nl802154.c:1584 genl_family_rcv_msg_doit+0x228/0x320 net/netlink/genetlink.c:739 genl_family_rcv_msg net/netlink/genetlink.c:783 [inline] genl_rcv_msg+0x328/0x580 net/netlink/genetlink.c:800 netlink_rcv_skb+0x153/0x420 net/netlink/af_netlink.c:2502 genl_rcv+0x24/0x40 net/netlink/genetlink.c:811 netlink_unicast_kernel net/netlink/af_netlink.c:1312 [inline] netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1338 netlink_sendmsg+0x856/0xd90 net/netlink/af_netlink.c:1927 sock_sendmsg_nosec net/socket.c:654 [inline] sock_sendmsg+0xcf/0x120 net/socket.c:674 ____sys_sendmsg+0x6e8/0x810 net/socket.c:2350 ___sys_sendmsg+0xf3/0x170 net/socket.c:2404 __sys_sendmsg+0xe5/0x1b0 net/socket.c:2433 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xae Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> Reported-by: syzbot+9ec037722d2603a9f52e@syzkaller.appspotmail.com Change-Id: I29f7ac641a039096d63d1e6070bb32cb5a3beb07 --- net/mac802154/llsec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)