Re: [PATCH net] ieee802154: lowpan_header_create check must check daddr

[Alexander Aring <aring@mojatatu.com>]

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 <willemb@google.com>
> 
> 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 <willemb@google.com>
> 

Acked-by: Alexander Aring <aring@mojatatu.com>

> ---
> 
> 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