[PATCH] net/ieee802154: reject zero-sized raw_sendmsg()

[PATCH] net/ieee802154: reject zero-sized raw_sendmsg()

[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+5ea725c25d06fb9114c4@syzkaller.appspotmail.com>
Fixes: fd1894224407c484 ("bpf: Don't redirect packets with invalid pkt_len")
Signed-off-by: Tetsuo Handa <penguin-kernel@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

[PATCH v2] net/ieee802154: don't warn zero-sized raw_sendmsg()

[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+5ea725c25d06fb9114c4@syzkaller.appspotmail.com>
Fixes: fd1894224407c484 ("bpf: Don't redirect packets with invalid pkt_len")
Signed-off-by: Tetsuo Handa <penguin-kernel@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

Re: [PATCH] net/ieee802154: reject zero-sized raw_sendmsg()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@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+5ea725c25d06fb9114c4@syzkaller.appspotmail.com>
> Fixes: fd1894224407c484 ("bpf: Don't redirect packets with invalid pkt_len")
> Signed-off-by: Tetsuo Handa <penguin-kernel@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

Re: [PATCH] net/ieee802154: reject zero-sized raw_sendmsg()

[Tetsuo Handa]

On 2022/10/03 21:30, patchwork-bot+netdevbpf@kernel.org wrote:
> Hello:
> 
> This patch was applied to netdev/net.git (master)
> by David S. Miller <davem@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+5ea725c25d06fb9114c4@syzkaller.appspotmail.com>
>> Fixes: fd1894224407c484 ("bpf: Don't redirect packets with invalid pkt_len")
>> Signed-off-by: Tetsuo Handa <penguin-kernel@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.

> 
> You are awesome, thank you!

Re: [PATCH] net/ieee802154: reject zero-sized raw_sendmsg()

[Alexander Aring]

Hi,

On Mon, Oct 3, 2022 at 8:35 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> On 2022/10/03 21:30, patchwork-bot+netdevbpf@kernel.org wrote:
> > Hello:
> >
> > This patch was applied to netdev/net.git (master)
> > by David S. Miller <davem@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+5ea725c25d06fb9114c4@syzkaller.appspotmail.com>
> >> Fixes: fd1894224407c484 ("bpf: Don't redirect packets with invalid pkt_len")
> >> Signed-off-by: Tetsuo Handa <penguin-kernel@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.

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

Re: [PATCH] net/ieee802154: reject zero-sized raw_sendmsg()

[Stefan Schmidt]

Hello.

On 04.10.22 00:29, Alexander Aring wrote:
> Hi,
> 
> On Mon, Oct 3, 2022 at 8:35 AM Tetsuo Handa
> <penguin-kernel@i-love.sakura.ne.jp> wrote:
>>
>> On 2022/10/03 21:30, patchwork-bot+netdevbpf@kernel.org wrote:
>>> Hello:
>>>
>>> This patch was applied to netdev/net.git (master)
>>> by David S. Miller <davem@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+5ea725c25d06fb9114c4@syzkaller.appspotmail.com>
>>>> Fixes: fd1894224407c484 ("bpf: Don't redirect packets with invalid pkt_len")
>>>> Signed-off-by: Tetsuo Handa <penguin-kernel@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.
> 
> I currently try to get access to kernel.org wpan repositories and try
> to rebase/apply your v2 on it. 

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

Re: [PATCH] net/ieee802154: reject zero-sized raw_sendmsg()

[Alexander Aring]

Hi,

On Tue, Oct 4, 2022 at 1:59 PM Stefan Schmidt <stefan@datenfreihafen.org> wrote:
>
> Hello.
>
> On 04.10.22 00:29, Alexander Aring wrote:
> > Hi,
> >
> > On Mon, Oct 3, 2022 at 8:35 AM Tetsuo Handa
> > <penguin-kernel@i-love.sakura.ne.jp> wrote:
> >>
> >> On 2022/10/03 21:30, patchwork-bot+netdevbpf@kernel.org wrote:
> >>> Hello:
> >>>
> >>> This patch was applied to netdev/net.git (master)
> >>> by David S. Miller <davem@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+5ea725c25d06fb9114c4@syzkaller.appspotmail.com>
> >>>> Fixes: fd1894224407c484 ("bpf: Don't redirect packets with invalid pkt_len")
> >>>> Signed-off-by: Tetsuo Handa <penguin-kernel@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.
> >
> > I currently try to get access to kernel.org wpan repositories and try
> > to rebase/apply your v2 on it.
>
> 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.
>

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

Re: [PATCH] net/ieee802154: reject zero-sized raw_sendmsg()

[Stefan Schmidt]

Hello Tetsuo.

On 03.10.22 14:34, Tetsuo Handa wrote:
> On 2022/10/03 21:30, patchwork-bot+netdevbpf@kernel.org wrote:
>> Hello:
>>
>> This patch was applied to netdev/net.git (master)
>> by David S. Miller <davem@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+5ea725c25d06fb9114c4@syzkaller.appspotmail.com>
>>> Fixes: fd1894224407c484 ("bpf: Don't redirect packets with invalid pkt_len")
>>> Signed-off-by: Tetsuo Handa <penguin-kernel@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.
> 

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

Re: [PATCH] net/ieee802154: reject zero-sized raw_sendmsg()

[Stefan Schmidt]

Hello.

On 05.10.22 03:49, Alexander Aring wrote:
> Hi,
> 
> On Tue, Oct 4, 2022 at 1:59 PM Stefan Schmidt <stefan@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