diff mbox series

net/ieee802154: reject zero-sized raw_sendmsg()

Message ID 5e89b653-3fc6-25c5-324b-1b15909c0183@I-love.SAKURA.ne.jp (mailing list archive)
State Accepted
Headers show
Series net/ieee802154: reject zero-sized raw_sendmsg() | expand

Commit Message

Tetsuo Handa Oct. 1, 2022, 4:43 p.m. UTC
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(+)

Comments

patchwork-bot+netdevbpf@kernel.org Oct. 3, 2022, 12:30 p.m. UTC | #1
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!
Tetsuo Handa Oct. 3, 2022, 12:34 p.m. UTC | #2
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!
Alexander Aring Oct. 3, 2022, 10:29 p.m. UTC | #3
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
Stefan Schmidt Oct. 4, 2022, 5:59 p.m. UTC | #4
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
Alexander Aring Oct. 5, 2022, 1:49 a.m. UTC | #5
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
Stefan Schmidt Oct. 5, 2022, 11:03 a.m. UTC | #6
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
Stefan Schmidt Oct. 5, 2022, 2:53 p.m. UTC | #7
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
diff mbox series

Patch

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);