* [PATCH net] packet: validate address length @ 2018-12-21 17:06 Willem de Bruijn 2018-12-21 17:11 ` David Miller 2018-12-22 15:39 ` Ido Schimmel 0 siblings, 2 replies; 4+ messages in thread From: Willem de Bruijn @ 2018-12-21 17:06 UTC (permalink / raw) To: netdev; +Cc: edumazet, Willem de Bruijn, syzbot From: Willem de Bruijn <willemb@google.com> Packet sockets with SOCK_DGRAM may pass an address for use in dev_hard_header. Ensure that it is of sufficient length. Reported-by: syzbot <syzkaller@googlegroups.com> Signed-off-by: Willem de Bruijn <willemb@google.com> --- net/packet/af_packet.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 6655793765b2d..5dda263b4a0a1 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -2627,6 +2627,8 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) proto = saddr->sll_protocol; addr = saddr->sll_addr; dev = dev_get_by_index(sock_net(&po->sk), saddr->sll_ifindex); + if (addr && dev && saddr->sll_halen < dev->addr_len) + goto out; } err = -ENXIO; @@ -2825,6 +2827,8 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len) proto = saddr->sll_protocol; addr = saddr->sll_addr; dev = dev_get_by_index(sock_net(sk), saddr->sll_ifindex); + if (addr && dev && saddr->sll_halen < dev->addr_len) + goto out; } err = -ENXIO; -- 2.20.1.415.g653613c723-goog ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net] packet: validate address length 2018-12-21 17:06 [PATCH net] packet: validate address length Willem de Bruijn @ 2018-12-21 17:11 ` David Miller 2018-12-22 15:39 ` Ido Schimmel 1 sibling, 0 replies; 4+ messages in thread From: David Miller @ 2018-12-21 17:11 UTC (permalink / raw) To: willemdebruijn.kernel; +Cc: netdev, edumazet, willemb, syzkaller From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: Fri, 21 Dec 2018 12:06:59 -0500 > From: Willem de Bruijn <willemb@google.com> > > Packet sockets with SOCK_DGRAM may pass an address for use in > dev_hard_header. Ensure that it is of sufficient length. > > Reported-by: syzbot <syzkaller@googlegroups.com> > Signed-off-by: Willem de Bruijn <willemb@google.com> Applied and queued up for -stable, thanks. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] packet: validate address length 2018-12-21 17:06 [PATCH net] packet: validate address length Willem de Bruijn 2018-12-21 17:11 ` David Miller @ 2018-12-22 15:39 ` Ido Schimmel 2018-12-22 20:05 ` Willem de Bruijn 1 sibling, 1 reply; 4+ messages in thread From: Ido Schimmel @ 2018-12-22 15:39 UTC (permalink / raw) To: Willem de Bruijn; +Cc: netdev, edumazet, Willem de Bruijn, syzbot On Fri, Dec 21, 2018 at 12:06:59PM -0500, Willem de Bruijn wrote: > From: Willem de Bruijn <willemb@google.com> > > Packet sockets with SOCK_DGRAM may pass an address for use in > dev_hard_header. Ensure that it is of sufficient length. > > Reported-by: syzbot <syzkaller@googlegroups.com> > Signed-off-by: Willem de Bruijn <willemb@google.com> Willem, We use mausezahn [1] in some of the tests under tools/testing/selftests/net/forwarding/ and I started observing failures today. Bisected it down to this patch. It seems that mausezahn passes 'sll_halen=0' [2]. Can you please take a look and adjust the check? Thanks [1] https://github.com/netsniff-ng/netsniff-ng [2] With patch: # strace -e network mausezahn dummy0 -c 1 -p 64 -a de:ad:be:ef:13:37 -t ip -q socket(AF_INET, SOCK_DGRAM, IPPROTO_IP) = 3 socket(AF_INET, SOCK_DGRAM, IPPROTO_IP) = 3 socket(AF_PACKET, SOCK_RAW, 768) = 3 setsockopt(3, SOL_SOCKET, SO_BROADCAST, [1], 4) = 0 socket(AF_INET, SOCK_DGRAM, IPPROTO_IP) = 4 socket(AF_INET, SOCK_DGRAM, IPPROTO_IP) = 4 socket(AF_INET, SOCK_DGRAM, IPPROTO_IP) = 3 socket(AF_PACKET, SOCK_RAW, 768) = 3 setsockopt(3, SOL_SOCKET, SO_BROADCAST, [1], 4) = 0 sendto(3, "\377\377\377\377\377\377\336\255\276\357\0237\10\0E\0\0T\0\0\0\0\377\0\273\252\377\377\377\377\377\377"..., 98, 0, {sa_family=AF_PACKET, sll_protocol=htons(ETH_P_ALL), sll_ifindex=if_nametoindex("dummy0"), sll_hatype=ARPHRD_NETROM, sll_pkttype=PACKET_HOST, sll_halen=0}, 20) = -1 EINVAL (Invalid argument) +++ exited with 0 +++ Without patch: # strace -e network mausezahn dummy0 -c 1 -p 64 -a de:ad:be:ef:13:37 -t ip -q socket(AF_INET, SOCK_DGRAM, IPPROTO_IP) = 3 socket(AF_INET, SOCK_DGRAM, IPPROTO_IP) = 3 socket(AF_PACKET, SOCK_RAW, 768) = 3 setsockopt(3, SOL_SOCKET, SO_BROADCAST, [1], 4) = 0 socket(AF_INET, SOCK_DGRAM, IPPROTO_IP) = 4 socket(AF_INET, SOCK_DGRAM, IPPROTO_IP) = 4 socket(AF_INET, SOCK_DGRAM, IPPROTO_IP) = 3 socket(AF_PACKET, SOCK_RAW, 768) = 3 setsockopt(3, SOL_SOCKET, SO_BROADCAST, [1], 4) = 0 sendto(3, "\377\377\377\377\377\377\336\255\276\357\0237\10\0E\0\0T\0\0\0\0\377\0\273\252\377\377\377\377\377\377"..., 98, 0, {sa_family=AF_PACKET, sll_protocol=htons(ETH_P_ALL), sll_ifindex=if_nametoindex("dummy0"), sll_hatype=ARPHRD_NETROM, sll_pkttype=PACKET_HOST, sll_halen=0}, 20) = 98 +++ exited with 0 +++ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] packet: validate address length 2018-12-22 15:39 ` Ido Schimmel @ 2018-12-22 20:05 ` Willem de Bruijn 0 siblings, 0 replies; 4+ messages in thread From: Willem de Bruijn @ 2018-12-22 20:05 UTC (permalink / raw) To: Ido Schimmel; +Cc: Network Development, Eric Dumazet, Willem de Bruijn, syzbot On Sat, Dec 22, 2018 at 10:39 AM Ido Schimmel <idosch@idosch.org> wrote: > > On Fri, Dec 21, 2018 at 12:06:59PM -0500, Willem de Bruijn wrote: > > From: Willem de Bruijn <willemb@google.com> > > > > Packet sockets with SOCK_DGRAM may pass an address for use in > > dev_hard_header. Ensure that it is of sufficient length. > > > > Reported-by: syzbot <syzkaller@googlegroups.com> > > Signed-off-by: Willem de Bruijn <willemb@google.com> > > Willem, > > We use mausezahn [1] in some of the tests under > tools/testing/selftests/net/forwarding/ and I started observing failures > today. Bisected it down to this patch. It seems that mausezahn passes > 'sll_halen=0' [2]. Can you please take a look and adjust the check? Thanks for the report, Ido. I should have checked for obvious case myself. Indeed the fix as I sent it is incorrect: saddr->sll_addr is not a pointer, but an array, so addr is never zero. This refinement should fix it and allow the tests to succeed. @@ -2825,7 +2825,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len) if (msg->msg_namelen < (saddr->sll_halen + offsetof(struct sockaddr_ll, sll_addr))) goto out; proto = saddr->sll_protocol; - addr = saddr->sll_addr; + addr = saddr->sll_halen ? saddr->sll_addr : 0; dev = dev_get_by_index(sock_net(sk), saddr->sll_ifindex); if (addr && dev && saddr->sll_halen < dev->addr_len) goto out; (and same in tpacket_snd) ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-12-22 20:05 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-12-21 17:06 [PATCH net] packet: validate address length Willem de Bruijn 2018-12-21 17:11 ` David Miller 2018-12-22 15:39 ` Ido Schimmel 2018-12-22 20:05 ` Willem de Bruijn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).