| Message ID | 20240213152414.3703-1-n.zhandarovich@fintech.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [wpan] mac802154: fix uninit-value issue in ieee802154_header_create() | expand |
Hi, On Tue, Feb 13, 2024 at 10:24 AM Nikita Zhandarovich <n.zhandarovich@fintech.ru> wrote: > > Syzkaller with KMSAN reported [1] a problem with uninitialized value > access in ieee802154_header_create(). > > The issue arises from a weird combination of cb->secen == 1 and > cb->secen_override == 0, while other required security parameters > are not found enabled in mac802154_set_header_security(). > In case of cb->secen is 1 and cb->secen_override is 0 mac802154_set_header_security() should depend on the ieee802154_llsec_params params. As [0] WPAN_SECURITY_DEFAULT signals this behaviour. > Ideally such case is expected to be caught by starting check at the > beginning of mac802154_set_header_security(): > > if (!params.enabled && cb->secen_override && cb->secen) > return -EINVAL; > > However, since secen_override is zero, the function in question > passes this check and returns with success early, without having > set values to ieee802154_sechdr fields such as key_id_mode. This in > turn leads to uninitialized access of such values in > ieee802154_hdr_push_sechdr() and other places. > > Fix this problem by only checking for secen value and presence of > security parameters (and ignoring secen_override). Exit early with > error if necessary requirements are not met. > > [1] > BUG: KMSAN: uninit-value in ieee802154_hdr_push_sechdr net/ieee802154/header_ops.c:54 [inline] > BUG: KMSAN: uninit-value in ieee802154_hdr_push+0x971/0xb90 net/ieee802154/header_ops.c:108 > ieee802154_hdr_push_sechdr net/ieee802154/header_ops.c:54 [inline] > ieee802154_hdr_push+0x971/0xb90 net/ieee802154/header_ops.c:108 > ieee802154_header_create+0x9c0/0xc00 net/mac802154/iface.c:396 > wpan_dev_hard_header include/net/cfg802154.h:494 [inline] > dgram_sendmsg+0xd1d/0x1500 net/ieee802154/socket.c:677 > ieee802154_sock_sendmsg+0x91/0xc0 net/ieee802154/socket.c:96 > sock_sendmsg_nosec net/socket.c:730 [inline] > __sock_sendmsg net/socket.c:745 [inline] > ____sys_sendmsg+0x9c2/0xd60 net/socket.c:2584 > ___sys_sendmsg+0x28d/0x3c0 net/socket.c:2638 > __sys_sendmsg net/socket.c:2667 [inline] > __do_sys_sendmsg net/socket.c:2676 [inline] > __se_sys_sendmsg net/socket.c:2674 [inline] > __x64_sys_sendmsg+0x307/0x490 net/socket.c:2674 > do_syscall_x64 arch/x86/entry/common.c:52 [inline] > do_syscall_64+0x44/0x110 arch/x86/entry/common.c:83 > entry_SYSCALL_64_after_hwframe+0x63/0x6b > > Local variable hdr created at: > ieee802154_header_create+0x4e/0xc00 net/mac802154/iface.c:360 > wpan_dev_hard_header include/net/cfg802154.h:494 [inline] > dgram_sendmsg+0xd1d/0x1500 net/ieee802154/socket.c:677 > > Fixes: f30be4d53cad ("mac802154: integrate llsec with wpan devices") > Reported-and-tested-by: syzbot+60a66d44892b66b56545@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=60a66d44892b66b56545 > Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru> > --- > P.S. Link to previous similar discussion: > https://lore.kernel.org/all/tencent_1C04CA8D66ADC45608D89687B4020B2A8706@qq.com/ > P.P.S. This issue may affect stable versions, at least up to 6.1. > > net/mac802154/iface.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c > index c0e2da5072be..ad799d349625 100644 > --- a/net/mac802154/iface.c > +++ b/net/mac802154/iface.c > @@ -328,7 +328,7 @@ static int mac802154_set_header_security(struct ieee802154_sub_if_data *sdata, > > mac802154_llsec_get_params(&sdata->sec, ¶ms); > > - if (!params.enabled && cb->secen_override && cb->secen) > + if (!params.enabled && cb->secen) > return -EINVAL; > if (!params.enabled || > (cb->secen_override && !cb->secen) || > I think there is just a missing check if (!cb->secen_override) then use whatever mac802154_llsec_get_params() says and ignore secen_enabled. Also I think that we don't init those socket parameters to any value at [1] so it's completely random what values are at socket creation. - Alex [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/ieee802154/socket.c?h=v6.8-rc5#n911 [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/ieee802154/socket.c?h=v6.8-rc5#n474
diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c index c0e2da5072be..ad799d349625 100644 --- a/net/mac802154/iface.c +++ b/net/mac802154/iface.c @@ -328,7 +328,7 @@ static int mac802154_set_header_security(struct ieee802154_sub_if_data *sdata, mac802154_llsec_get_params(&sdata->sec, ¶ms); - if (!params.enabled && cb->secen_override && cb->secen) + if (!params.enabled && cb->secen) return -EINVAL; if (!params.enabled || (cb->secen_override && !cb->secen) ||
Syzkaller with KMSAN reported [1] a problem with uninitialized value access in ieee802154_header_create(). The issue arises from a weird combination of cb->secen == 1 and cb->secen_override == 0, while other required security parameters are not found enabled in mac802154_set_header_security(). Ideally such case is expected to be caught by starting check at the beginning of mac802154_set_header_security(): if (!params.enabled && cb->secen_override && cb->secen) return -EINVAL; However, since secen_override is zero, the function in question passes this check and returns with success early, without having set values to ieee802154_sechdr fields such as key_id_mode. This in turn leads to uninitialized access of such values in ieee802154_hdr_push_sechdr() and other places. Fix this problem by only checking for secen value and presence of security parameters (and ignoring secen_override). Exit early with error if necessary requirements are not met. [1] BUG: KMSAN: uninit-value in ieee802154_hdr_push_sechdr net/ieee802154/header_ops.c:54 [inline] BUG: KMSAN: uninit-value in ieee802154_hdr_push+0x971/0xb90 net/ieee802154/header_ops.c:108 ieee802154_hdr_push_sechdr net/ieee802154/header_ops.c:54 [inline] ieee802154_hdr_push+0x971/0xb90 net/ieee802154/header_ops.c:108 ieee802154_header_create+0x9c0/0xc00 net/mac802154/iface.c:396 wpan_dev_hard_header include/net/cfg802154.h:494 [inline] dgram_sendmsg+0xd1d/0x1500 net/ieee802154/socket.c:677 ieee802154_sock_sendmsg+0x91/0xc0 net/ieee802154/socket.c:96 sock_sendmsg_nosec net/socket.c:730 [inline] __sock_sendmsg net/socket.c:745 [inline] ____sys_sendmsg+0x9c2/0xd60 net/socket.c:2584 ___sys_sendmsg+0x28d/0x3c0 net/socket.c:2638 __sys_sendmsg net/socket.c:2667 [inline] __do_sys_sendmsg net/socket.c:2676 [inline] __se_sys_sendmsg net/socket.c:2674 [inline] __x64_sys_sendmsg+0x307/0x490 net/socket.c:2674 do_syscall_x64 arch/x86/entry/common.c:52 [inline] do_syscall_64+0x44/0x110 arch/x86/entry/common.c:83 entry_SYSCALL_64_after_hwframe+0x63/0x6b Local variable hdr created at: ieee802154_header_create+0x4e/0xc00 net/mac802154/iface.c:360 wpan_dev_hard_header include/net/cfg802154.h:494 [inline] dgram_sendmsg+0xd1d/0x1500 net/ieee802154/socket.c:677 Fixes: f30be4d53cad ("mac802154: integrate llsec with wpan devices") Reported-and-tested-by: syzbot+60a66d44892b66b56545@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=60a66d44892b66b56545 Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru> --- P.S. Link to previous similar discussion: https://lore.kernel.org/all/tencent_1C04CA8D66ADC45608D89687B4020B2A8706@qq.com/ P.P.S. This issue may affect stable versions, at least up to 6.1. net/mac802154/iface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)