Re: [PATCH net] packet: validate address length

[Willem de Bruijn]

On Sat, Dec 22, 2018 at 10:39 AM Ido Schimmel <ido...@idosch.org> wrote:
>
> On Fri, Dec 21, 2018 at 12:06:59PM -0500, Willem de Bruijn wrote:
> > From: Willem de Bruijn <will...@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 <syzkal...@googlegroups.com>
> > Signed-off-by: Willem de Bruijn <will...@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-&gt;msg_namelen &lt; (saddr-&gt;sll_halen +
offsetof(struct sockaddr_ll, sll_addr)))
                        goto out;
                proto   = saddr-&gt;sll_protocol;
-               addr    = saddr-&gt;sll_addr;
+               addr    = saddr-&gt;sll_halen ? saddr-&gt;sll_addr : 0;
                dev = dev_get_by_index(sock_net(sk), saddr-&gt;sll_ifindex);
                if (addr &amp;&amp; dev &amp;&amp; saddr-&gt;sll_halen
&lt; dev-&gt;addr_len)
                        goto out;

(and same in tpacket_snd)