From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Aring Subject: Re: [PATCH net] ieee802154: lowpan_header_create check must check daddr Date: Sun, 23 Dec 2018 15:45:47 -0500 Message-ID: <20181223204547.aqtr7jxwzdvteao6@x220t> References: <20181223175218.165576-1-willemdebruijn.kernel@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: netdev@vger.kernel.org, davem@davemloft.net, alex.aring@gmail.com, jukka.rissanen@linux.intel.com, Willem de Bruijn To: Willem de Bruijn Return-path: Received: from mail-io1-f66.google.com ([209.85.166.66]:39212 "EHLO mail-io1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725850AbeLWUpx (ORCPT ); Sun, 23 Dec 2018 15:45:53 -0500 Received: by mail-io1-f66.google.com with SMTP id k7so7784018iob.6 for ; Sun, 23 Dec 2018 12:45:53 -0800 (PST) Content-Disposition: inline In-Reply-To: <20181223175218.165576-1-willemdebruijn.kernel@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: Hi, thanks Willem to take a look into these callbacks. On Sun, Dec 23, 2018 at 12:52:18PM -0500, Willem de Bruijn wrote: > From: Willem de Bruijn > > Packet sockets may call dev_header_parse with NULL daddr. Make > lowpan_header_ops.create fail. > Ok. > Fixes: 87a93e4eceb4 ("ieee802154: change needed headroom/tailroom") > Signed-off-by: Willem de Bruijn > Acked-by: Alexander Aring > --- > > Re: function comment on packet socket address length: that is (now) > verified to be at least dev->addr_len. > I had some questions when I was digging AF_PACKET code. So the UAPI limitation of AF_PACKET has a sockaddr_t of 8 bytes. What is when I assign e.g. more than 8 bytes to dev->addr_len and copying dev->addr_len to it. Does we care about that? At least some assert warning if somebody try to use larger than 8 bytes dev->addr_len for AF_PACKET dgram sockets which might using these pointers and copy dev->addr_len size? As I already saw it before, but don't know what the best place it is to check on that. > It is customary to return -header_len on failure in .create(), but > not sure what that would be here, and any negative value is treated > the same by callers, so returning -EINVAL. > > Is the return 0 on !ETH_P_IPV6 intentional, or should that also be > negative? Should be, maybe not supported. The function of a lowpan device here is just header "transforming". I used "transforming" here because it's still an IPv6 header afterwards (or more) current case is more compression only. I need to admit, I never tried AF_PACKET on a lowpan interface but I thought about it that it ends in bad things... I would like to forbid it, because they should use RAW IPv6 sockets where at least we already have code to check that we have at least a IPv6 header at skb_packer_header() (I hope this is how it works). Is there any way to do that? - Alex