[syzbot] WARNING in __dev_queue_xmit

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: b77ffb30cfc5 libbpf: fix an snprintf() overflow check
git tree: bpf-next
console+strace: https://syzkaller.appspot.com/x/log.txt?x=11ef3226080000
kernel config: https://syzkaller.appspot.com/x/.config?x=386b986585586629
dashboard link: https://syzkaller.appspot.com/bug?extid=5ea725c25d06fb9114c4
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=11004e64080000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1009bfd0080000

The issue was bisected to:

commit fd1894224407c484f652ad456e1ce423e89bb3eb
Author: Zhengchao Shao <shaozh...@huawei.com>
Date: Fri Jul 15 11:55:59 2022 +0000

bpf: Don't redirect packets with invalid pkt_len

bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1749bfd0080000
final oops: https://syzkaller.appspot.com/x/report.txt?x=14c9bfd0080000
console output: https://syzkaller.appspot.com/x/log.txt?x=10c9bfd0080000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+5ea725...@syzkaller.appspotmail.com
Fixes: fd1894224407 ("bpf: Don't redirect packets with invalid pkt_len")

------------[ cut here ]------------
skb_assert_len
WARNING: CPU: 0 PID: 3608 at include/linux/skbuff.h:2465 skb_assert_len include/linux/skbuff.h:2465 [inline]
WARNING: CPU: 0 PID: 3608 at include/linux/skbuff.h:2465 __dev_queue_xmit+0x313c/0x3ad0 net/core/dev.c:4171
Modules linked in:
CPU: 0 PID: 3608 Comm: syz-executor357 Not tainted 5.19.0-rc5-syzkaller-01146-gb77ffb30cfc5 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 06/29/2022
RIP: 0010:skb_assert_len include/linux/skbuff.h:2465 [inline]
RIP: 0010:__dev_queue_xmit+0x313c/0x3ad0 net/core/dev.c:4171
Code: 10 e8 18 6c 73 fa e9 b4 f2 ff ff e8 4e 18 26 fa 48 c7 c6 a0 81 d3 8a 48 c7 c7 a0 55 d3 8a c6 05 89 33 52 06 01 e8 45 75 de 01 <0f> 0b e9 4f f2 ff ff e8 28 18 26 fa e8 a3 17 10 fa 31 ff 89 c3 89
RSP: 0018:ffffc900031af748 EFLAGS: 00010286
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: ffff88801b9bbb00 RSI: ffffffff8160d618 RDI: fffff52000635edb
RBP: ffff888020059aba R08: 0000000000000005 R09: 0000000000000000
R10: 0000000080000000 R11: 0000000000000001 R12: ffff888020059a00
R13: 0000000000000000 R14: ffff888020059a10 R15: ffff888020059a00
FS: 0000555556f80300(0000) GS:ffff8880b9a00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000561be8192048 CR3: 0000000020721000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
packet_snd net/packet/af_packet.c:3073 [inline]
packet_sendmsg+0x21f4/0x55d0 net/packet/af_packet.c:3104
sock_sendmsg_nosec net/socket.c:714 [inline]
sock_sendmsg+0xcf/0x120 net/socket.c:734
____sys_sendmsg+0x6eb/0x810 net/socket.c:2485
___sys_sendmsg+0xf3/0x170 net/socket.c:2539
__sys_sendmsg net/socket.c:2568 [inline]
__do_sys_sendmsg net/socket.c:2577 [inline]
__se_sys_sendmsg net/socket.c:2575 [inline]
__x64_sys_sendmsg+0x132/0x220 net/socket.c:2575
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+0x46/0xb0
RIP: 0033:0x7f4882617369
Code: 28 c3 e8 4a 15 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:00007fff94d23dd8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007f4882617369
RDX: 0000000000000000 RSI: 0000000020000100 RDI: 0000000000000003
RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007fff94d23df0
R13: 00007fff94d23e10 R14: 0000000000000000 R15: 0000000000000000
</TASK>


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

[Tetsuo Handa]

syzbot is hitting skb_assert_len() warning at raw_sendmsg() for ieee802154
socket. What commit dc633700f00f726e ("net/af_packet: check len when
min_header_len equals to 0") does also applies to ieee802154 socket.

Link: https://syzkaller.appspot.com/bug?extid=5ea725c25d06fb9114c4
Reported-by: syzbot <syzbot+5ea725...@syzkaller.appspotmail.com>
Fixes: fd1894224407c484 ("bpf: Don't redirect packets with invalid pkt_len")
Signed-off-by: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
---
I checked that reproducer no longer hits skb_assert_len() warning, but
what return value should we use? Is -EDESTADDRREQ better than -EINVAL?

net/ieee802154/socket.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
index 7889e1ef7fad..cbd0e2ac4ffe 100644
--- a/net/ieee802154/socket.c
+++ b/net/ieee802154/socket.c
@@ -251,6 +251,9 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
return -EOPNOTSUPP;
}

+ if (!size)
+ return -EINVAL;
+
lock_sock(sk);
if (!sk->sk_bound_dev_if)
dev = dev_getfirstbyhwtype(sock_net(sk), ARPHRD_IEEE802154);
--
2.34.1

[Tetsuo Handa]

syzbot is hitting skb_assert_len() warning at __dev_queue_xmit() [1],
for PF_IEEE802154 socket's zero-sized raw_sendmsg() request is hitting
__dev_queue_xmit() with skb->len == 0.

Since PF_IEEE802154 socket's zero-sized raw_sendmsg() request was
able to return 0, don't call __dev_queue_xmit() if packet length is 0.

----------
#include <sys/socket.h>
#include <netinet/in.h>

int main(int argc, char *argv[])
{
struct sockaddr_in addr = { .sin_family = AF_INET, .sin_addr.s_addr = htonl(INADDR_LOOPBACK) };
struct iovec iov = { };
struct msghdr hdr = { .msg_name = &addr, .msg_namelen = sizeof(addr), .msg_iov = &iov, .msg_iovlen = 1 };
sendmsg(socket(PF_IEEE802154, SOCK_RAW, 0), &hdr, 0);
return 0;
}
----------

Note that this might be a sign that commit fd1894224407c484 ("bpf: Don't
redirect packets with invalid pkt_len") should be reverted, for
skb->len == 0 was acceptable for at least PF_IEEE802154 socket.

Link: https://syzkaller.appspot.com/bug?extid=5ea725c25d06fb9114c4 [1]

Reported-by: syzbot <syzbot+5ea725...@syzkaller.appspotmail.com>
Fixes: fd1894224407c484 ("bpf: Don't redirect packets with invalid pkt_len")
Signed-off-by: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
---

Changes in v2:
Return 0 after checking -ENXIO case, for a test program shown above
returns -ENXIO if dev was not found and returns 0 otherwise.

net/ieee802154/socket.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
index 7889e1ef7fad..6e55fae4c686 100644
--- a/net/ieee802154/socket.c
+++ b/net/ieee802154/socket.c
@@ -272,6 +272,10 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
err = -EMSGSIZE;
goto out_dev;
}
+ if (!size) {
+ err = 0;
+ goto out_dev;
+ }

hlen = LL_RESERVED_SPACE(dev);
tlen = dev->needed_tailroom;
--
2.34.1

[patchwork-b...@kernel.org]

Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <da...@davemloft.net>:

On Sun, 2 Oct 2022 01:43:44 +0900 you wrote:
> syzbot is hitting skb_assert_len() warning at raw_sendmsg() for ieee802154
> socket. What commit dc633700f00f726e ("net/af_packet: check len when
> min_header_len equals to 0") does also applies to ieee802154 socket.
>
> Link: https://syzkaller.appspot.com/bug?extid=5ea725c25d06fb9114c4
> Reported-by: syzbot <syzbot+5ea725...@syzkaller.appspotmail.com>
> Fixes: fd1894224407c484 ("bpf: Don't redirect packets with invalid pkt_len")
> Signed-off-by: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
>

> [...]

Here is the summary with links:
- net/ieee802154: reject zero-sized raw_sendmsg()
https://git.kernel.org/netdev/net/c/3a4d061c699b

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

[Tetsuo Handa]

On 2022/10/03 21:30, patchwork-b...@kernel.org wrote:
> Hello:
>
> This patch was applied to netdev/net.git (master)
> by David S. Miller <da...@davemloft.net>:
>
> On Sun, 2 Oct 2022 01:43:44 +0900 you wrote:
>> syzbot is hitting skb_assert_len() warning at raw_sendmsg() for ieee802154
>> socket. What commit dc633700f00f726e ("net/af_packet: check len when
>> min_header_len equals to 0") does also applies to ieee802154 socket.
>>
>> Link: https://syzkaller.appspot.com/bug?extid=5ea725c25d06fb9114c4
>> Reported-by: syzbot <syzbot+5ea725...@syzkaller.appspotmail.com>
>> Fixes: fd1894224407c484 ("bpf: Don't redirect packets with invalid pkt_len")
>> Signed-off-by: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
>>
>> [...]
>
> Here is the summary with links:
> - net/ieee802154: reject zero-sized raw_sendmsg()
> https://git.kernel.org/netdev/net/c/3a4d061c699b

Are you sure that returning -EINVAL is OK?

In v2 patch, I changed to return 0, for PF_IEEE802154 socket's zero-sized
raw_sendmsg() request was able to return 0.

[Alexander Aring]

Hi,

I currently try to get access to kernel.org wpan repositories and try
to rebase/apply your v2 on it. Then it should be fixed in the next
pull request to net. For netdev maintainers, please don't apply wpan
patches. Stefan and I will care about it.

Thanks.

- Alex

[Stefan Schmidt]

Hello.

This will only work once I merged net into wpan. Which I normally do
only after a pull request to avoid merge requests being created.

We have two options here a) reverting this patch and applying v2 of it
b) Tetsu sending an incremental patch on top of the applied one to come
to the same state as after v2.

Then it should be fixed in the next
> pull request to net. For netdev maintainers, please don't apply wpan
> patches. Stefan and I will care about it.

Keep in mind that Dave and Jakub do this to help us out because we are
sometimes slow on applying patches and getting them to net. Normally
this is all fine for clear fixes.

For -next material I agree this should only go through the wpan-next
tree for us to coordinate, but for the occasional fix its often faster
if it hits net directly. Normally I don't mind that. In this case v2 was
overlooked. But this is easily rectified with either of the two options
mentioned above.

regards
Stefan Schmidt

[Alexander Aring]

Hi,

ok.

> We have two options here a) reverting this patch and applying v2 of it
> b) Tetsu sending an incremental patch on top of the applied one to come
> to the same state as after v2.
>
>
> Then it should be fixed in the next

ok.

> > pull request to net. For netdev maintainers, please don't apply wpan
> > patches. Stefan and I will care about it.
>
> Keep in mind that Dave and Jakub do this to help us out because we are
> sometimes slow on applying patches and getting them to net. Normally
> this is all fine for clear fixes.
>

If we move getting patches for wpan to net then we should move it
completely to that behaviour and not having a mixed setup which does
not work, or it works and hope we don't have conflicts and if we have
conflicts we need to fix them when doing the pull-request that the
next instance has no conflicts because they touched maybe the same
code area.

> For -next material I agree this should only go through the wpan-next
> tree for us to coordinate, but for the occasional fix its often faster
> if it hits net directly. Normally I don't mind that. In this case v2 was
> overlooked. But this is easily rectified with either of the two options
> mentioned above.
>

I think a) would be the fastest way here and I just sent something.

- Alex

[Stefan Schmidt]

Hello Tetsuo.

I pulled in the revert from Alex and your v2 patch into my wpan tree
now. It will go to net from there.

regards
Stefan Schmidt

[Stefan Schmidt]

Hello.

On 05.10.22 03:49, Alexander Aring wrote:
> Hi,
>
> On Tue, Oct 4, 2022 at 1:59 PM Stefan Schmidt <ste...@datenfreihafen.org> wrote:
>>
>> Hello.
>>
>> On 04.10.22 00:29, Alexander Aring wrote:
>>> pull request to net. For netdev maintainers, please don't apply wpan
>>> patches. Stefan and I will care about it.
>>
>> Keep in mind that Dave and Jakub do this to help us out because we are
>> sometimes slow on applying patches and getting them to net. Normally
>> this is all fine for clear fixes.
>>
>
> If we move getting patches for wpan to net then we should move it
> completely to that behaviour and not having a mixed setup which does
> not work, or it works and hope we don't have conflicts and if we have
> conflicts we need to fix them when doing the pull-request that the
> next instance has no conflicts because they touched maybe the same
> code area.

I do disagree on this. I think there is no need to have it fixed to one
way or another (net OR wpan). It has been working fine with this mixed
approach for quite a long time. The current issue with v1 being applied
instead of v2 is something that could have happened to us when applying
to wpan as easily.

If we are quick enough to ack/apply patches hitting the list (1-2 days)
its unlikely any of them will be applied to net. Dave and Jakub simply
help us to make sure nothing falls through the cracks.

> I think a) would be the fastest way here and I just sent something.

I applied the two patches earlier today and just send out a pull request
for net with them.

regards
Stefan Schmidt