netdev - RE: [PATCH net] packet: validate address length if non-zero

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <28df499bec4146fdbd9d66a6bf7dc621@AcuMS.aculab.com>
Date:   Tue, 23 Apr 2019 10:00:15 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Willem de Bruijn' <willemdebruijn.kernel@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "idosch@...sch.org" <idosch@...sch.org>,
        Willem de Bruijn <willemb@...gle.com>
Subject: RE: [PATCH net] packet: validate address length if non-zero

From: Willem de Bruijn
> Sent: 22 December 2018 21:54
> Validate packet socket address length if a length is given. Zero
> length is equivalent to not setting an address.
> 
> Fixes: 99137b7888f4 ("packet: validate address length")
> Reported-by: Ido Schimmel <idosch@...sch.org>
> Signed-off-by: Willem de Bruijn <willemb@...gle.com>
> ---
>  net/packet/af_packet.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 5dda263b4a0a..eedacdebcd4c 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -2625,7 +2625,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
>  						sll_addr)))
>  			goto out;
>  		proto	= saddr->sll_protocol;
> -		addr	= saddr->sll_addr;
> +		addr	= saddr->sll_halen ? saddr->sll_addr : NULL;
>  		dev = dev_get_by_index(sock_net(&po->sk), saddr->sll_ifindex);
>  		if (addr && dev && saddr->sll_halen < dev->addr_len)
>  			goto out;
> @@ -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 : NULL;
>  		dev = dev_get_by_index(sock_net(sk), saddr->sll_ifindex);
>  		if (addr && dev && saddr->sll_halen < dev->addr_len)
>  			goto out;
> --
> 2.20.1.415.g653613c723-goog

We've just discovered the combination of this patch and the one it 'fixes'
breaks some of our userspace code.

Prior to these changes it didn't matter if code using AF_PACKET to
send ethernet frames on a specific 'ethertype' failed to set sll_addr.
Everything assumed it would be 6 - and the packets were sent.

With both changes you get a -EINVAL return from somewhere.
I can fix our code, but I doubt it is the only code affected.
Other people are likely to have copied the same example.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)