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 |
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!
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!
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
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
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
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
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 --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);
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(+)