From: Kees Cook <keescook@chromium.org>
To: Hawkins Jiawei <yin31149@gmail.com>
Cc: syzbot+473754e5af963cf014cf@syzkaller.appspotmail.com,
Johannes Berg <johannes@sipsolutions.net>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, syzkaller-bugs@googlegroups.com,
18801353760@163.com, Stephen Rothwell <sfr@canb.auug.org.au>
Subject: Re: [PATCH] Add linux-next specific files for 20220923
Date: Sat, 24 Sep 2022 00:26:08 -0700 [thread overview]
Message-ID: <202209240014.C7B6E929A@keescook> (raw)
In-Reply-To: <20220924071035.16027-1-yin31149@gmail.com>
On Sat, Sep 24, 2022 at 03:10:34PM +0800, Hawkins Jiawei wrote:
> From: Stephen Rothwell <sfr@canb.auug.org.au>
>
> > Hello,
> >
> > syzbot found the following issue on:
> >
> > HEAD commit: 483fed3b5dc8 Add linux-next specific files for 20220921
> > git tree: linux-next
> > console+strace: https://syzkaller.appspot.com/x/log.txt?x=1154ddd5080000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=849cb9f70f15b1ba
> > dashboard link: https://syzkaller.appspot.com/bug?extid=473754e5af963cf014cf
> > compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=157c196f080000
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11f12618880000
> >
> > Downloadable assets:
> > disk image: https://storage.googleapis.com/syzbot-assets/1cb3f4618323/disk-483fed3b.raw.xz
> > vmlinux: https://storage.googleapis.com/syzbot-assets/cc02cb30b495/vmlinux-483fed3b.xz
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+473754e5af963cf014cf@syzkaller.appspotmail.com
> >
> > ------------[ cut here ]------------
> > memcpy: detected field-spanning write (size 8) of single field "&compat_event->pointer" at net/wireless/wext-core.c:623 (size 4)
> > WARNING: CPU: 0 PID: 3607 at net/wireless/wext-core.c:623 wireless_send_event+0xab5/0xca0 net/wireless/wext-core.c:623
> > Modules linked in:
> > CPU: 1 PID: 3607 Comm: syz-executor659 Not tainted 6.0.0-rc6-next-20220921-syzkaller #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/16/2022
> > RIP: 0010:wireless_send_event+0xab5/0xca0 net/wireless/wext-core.c:623
> > Code: fa ff ff e8 cd b9 db f8 b9 04 00 00 00 4c 89 e6 48 c7 c2 e0 56 11 8b 48 c7 c7 20 56 11 8b c6 05 94 8e 2a 05 01 e8 b8 b0 a6 00 <0f> 0b e9 9b fa ff ff e8 6f ef 27 f9 e9 a6 fd ff ff e8 c5 ef 27 f9
> > RSP: 0018:ffffc90003b2fbc0 EFLAGS: 00010286
> > RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
> > RDX: ffff888021d157c0 RSI: ffffffff81620348 RDI: fffff52000765f6a
> > RBP: ffff88801e15c780 R08: 0000000000000005 R09: 0000000000000000
> > R10: 0000000080000000 R11: 20676e696e6e6170 R12: 0000000000000008
> > R13: ffff888025a72640 R14: ffff8880225d402c R15: ffff8880225d4034
> > FS: 0000555556bd9300(0000) GS:ffff8880b9b00000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 00007fbda677dfb8 CR3: 000000007b976000 CR4: 00000000003506e0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > Call Trace:
> > <TASK>
> > ioctl_standard_call+0x155/0x1f0 net/wireless/wext-core.c:1022
> > wireless_process_ioctl+0xc8/0x4c0 net/wireless/wext-core.c:955
> > wext_ioctl_dispatch net/wireless/wext-core.c:988 [inline]
> > wext_ioctl_dispatch net/wireless/wext-core.c:976 [inline]
> > wext_handle_ioctl+0x26b/0x280 net/wireless/wext-core.c:1049
> > sock_ioctl+0x285/0x640 net/socket.c:1220
> > vfs_ioctl fs/ioctl.c:51 [inline]
> > __do_sys_ioctl fs/ioctl.c:870 [inline]
> > __se_sys_ioctl fs/ioctl.c:856 [inline]
> > __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:856
> > do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> > do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
> > entry_SYSCALL_64_after_hwframe+0x63/0xcd
> > RIP: 0033:0x7fbda6736af9
> > Code: 28 c3 e8 2a 14 00 00 66 2e 0f 1f 84 00 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
> > RSP: 002b:00007ffd45e80138 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> > RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fbda6736af9
> > RDX: 0000000020000000 RSI: 0000000000008b04 RDI: 0000000000000003
> > RBP: 00007fbda66faca0 R08: 0000000000000000 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000246 R12: 00007fbda66fad30
> > R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> > </TASK>
>
> I think this is the samiliar problem as what Kees Cook pointed out in
> https://lore.kernel.org/linux-next/202209211250.3049C29@keescook/
>
> It seems that memcpy() will performs run-time buffer bounds
> checking, which triggers this warning.
>
> #syz test git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> master
>
>
> diff --git a/include/linux/wireless.h b/include/linux/wireless.h
> index 2d1b54556eff..81603848b0aa 100644
> --- a/include/linux/wireless.h
> +++ b/include/linux/wireless.h
> @@ -26,7 +26,10 @@ struct compat_iw_point {
> struct __compat_iw_event {
> __u16 len; /* Real length of this stuff */
> __u16 cmd; /* Wireless IOCTL */
> - compat_caddr_t pointer;
> + union {
> + compat_caddr_t pointer;
> + __DECLARE_FLEX_ARRAY(__u8, pointer_flex);
> + };
Is this expected to be dynamically sized? I assume so, given the "Real
length" comment. :)
> };
> #define IW_EV_COMPAT_LCP_LEN offsetof(struct __compat_iw_event, pointer)
> #define IW_EV_COMPAT_POINT_OFF offsetof(struct compat_iw_point, length)
> diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c
> index 76a80a41615b..9d0b50abbe09 100644
> --- a/net/wireless/wext-core.c
> +++ b/net/wireless/wext-core.c
> @@ -620,7 +620,7 @@ void wireless_send_event(struct net_device * dev,
adding in more context code:
memcpy(&compat_event->pointer,
((char *) &compat_wrqu) + IW_EV_COMPAT_POINT_OFF,
hdr_len - IW_EV_COMPAT_LCP_LEN);
if (extra_len)
memcpy(((char *) compat_event) + hdr_len,
extra, extra_len);
The code above has "pointer" as a memcpy destination as well. I think
that should be changed to pointer_flex as well, as the length calculation
is the same. I wonder what FORTIFY will think about the second memcpy
above. If I'm reading the math correctly, it might need to be:
if (extra_len) {
size_t offset = hdr_len - offsetof(typeof(*compat_event), pointer_flex);
memcpy(&compat_event->pointer_flex[offset], extra, extra_len);
}
> } else {
> /* extra_len must be zero, so no if (extra) needed */
> - memcpy(&compat_event->pointer, wrqu,
> + memcpy(&compat_event->pointer_flex, wrqu,
> hdr_len - IW_EV_COMPAT_LCP_LEN);
> }
>
But otherwise, yes, looks like the right modification. Thanks for tackling
this! It is quite a weird structure! :)
-Kees
--
Kees Cook
next prev parent reply other threads:[~2022-09-24 7:27 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-23 16:05 [syzbot] WARNING in wireless_send_event syzbot
2022-09-24 7:10 ` [PATCH] Add linux-next specific files for 20220923 Hawkins Jiawei
2022-09-24 7:26 ` Kees Cook [this message]
2022-09-24 11:44 ` Hawkins Jiawei
2022-09-24 15:55 ` Hawkins Jiawei
2022-09-24 15:55 ` syzbot
2022-09-24 16:13 ` Hawkins Jiawei
2022-09-24 16:33 ` [syzbot] WARNING in wireless_send_event syzbot
2022-09-24 16:26 ` [PATCH] Add linux-next specific files for 20220923 Kees Cook
2022-09-25 6:25 ` Hawkins Jiawei
2022-09-24 7:32 ` [syzbot] WARNING in wireless_send_event syzbot
2022-09-26 11:59 ` Hawkins Jiawei
2022-09-26 12:24 ` syzbot
2022-09-26 15:09 ` [PATCH] wext: use flex array destination for memcpy() Hawkins Jiawei
2022-09-26 16:14 ` Eric Dumazet
2022-09-26 23:17 ` Hawkins Jiawei
2022-09-26 17:43 ` Kees Cook
2022-09-26 23:34 ` [PATCH wireless-next v2] " Hawkins Jiawei
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202209240014.C7B6E929A@keescook \
--to=keescook@chromium.org \
--cc=18801353760@163.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=johannes@sipsolutions.net \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sfr@canb.auug.org.au \
--cc=syzbot+473754e5af963cf014cf@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=yin31149@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.