From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2F26EC10F14 for ; Tue, 23 Apr 2019 09:59:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0697D21738 for ; Tue, 23 Apr 2019 09:59:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727180AbfDWJ7L convert rfc822-to-8bit (ORCPT ); Tue, 23 Apr 2019 05:59:11 -0400 Received: from eu-smtp-delivery-151.mimecast.com ([146.101.78.151]:50990 "EHLO eu-smtp-delivery-151.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726443AbfDWJ7H (ORCPT ); Tue, 23 Apr 2019 05:59:07 -0400 Received: from AcuMS.aculab.com (156.67.243.126 [156.67.243.126]) (Using TLS) by relay.mimecast.com with ESMTP id uk-mta-187-2vDpZ4krOUWWZRZrci0-qg-1; Tue, 23 Apr 2019 10:59:03 +0100 Received: from AcuMS.Aculab.com (fd9f:af1c:a25b:0:43c:695e:880f:8750) by AcuMS.aculab.com (fd9f:af1c:a25b:0:43c:695e:880f:8750) with Microsoft SMTP Server (TLS) id 15.0.1347.2; Tue, 23 Apr 2019 11:00:15 +0100 Received: from AcuMS.Aculab.com ([fe80::43c:695e:880f:8750]) by AcuMS.aculab.com ([fe80::43c:695e:880f:8750%12]) with mapi id 15.00.1347.000; Tue, 23 Apr 2019 11:00:15 +0100 From: David Laight To: 'Willem de Bruijn' , "netdev@vger.kernel.org" CC: "davem@davemloft.net" , "idosch@idosch.org" , Willem de Bruijn Subject: RE: [PATCH net] packet: validate address length if non-zero Thread-Topic: [PATCH net] packet: validate address length if non-zero Thread-Index: AQHUmkDdSvpkaV9kXkOyGO8KJ3nLv6ZKP8XA Date: Tue, 23 Apr 2019 10:00:15 +0000 Message-ID: <28df499bec4146fdbd9d66a6bf7dc621@AcuMS.aculab.com> References: <20181222215345.128704-1-willemdebruijn.kernel@gmail.com> In-Reply-To: <20181222215345.128704-1-willemdebruijn.kernel@gmail.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.202.205.107] MIME-Version: 1.0 X-MC-Unique: 2vDpZ4krOUWWZRZrci0-qg-1 X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 > Signed-off-by: Willem de Bruijn > --- > 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)