netdev - Re: [PATCH net] ieee802154: lowpan_header

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]

Date:   Sun, 23 Dec 2018 15:45:47 -0500
From:   Alexander Aring <aring@...atatu.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, alex.aring@...il.com,
        jukka.rissanen@...ux.intel.com,
        Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net] ieee802154: lowpan_header_create check must check
 daddr

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@...gle.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@...gle.com>
> 

Acked-by: Alexander Aring <aring@...atatu.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