netdev - Re: [PATCH net-next 3/4] net, neigh: Extend neigh->flags to 32 bit to allow for extensions

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: <0b1794ec-f55d-3485-d192-e6704af8273d@iogearbox.net>
Date:   Tue, 12 Oct 2021 16:46:13 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     David Ahern <dsahern@...il.com>, davem@...emloft.net,
        kuba@...nel.org
Cc:     roopa@...dia.com, dsahern@...nel.org, m@...bda.lt,
        john.fastabend@...il.com, netdev@...r.kernel.org,
        bpf@...r.kernel.org
Subject: Re: [PATCH net-next 3/4] net, neigh: Extend neigh->flags to 32 bit to
 allow for extensions

On 10/12/21 4:31 PM, David Ahern wrote:
> On 10/11/21 6:12 AM, Daniel Borkmann wrote:
>> diff --git a/include/net/neighbour.h b/include/net/neighbour.h
>> index eb2a7c03a5b0..26d4ada0aea9 100644
>> --- a/include/net/neighbour.h
>> +++ b/include/net/neighbour.h
>> @@ -144,11 +144,11 @@ struct neighbour {
>>   	struct timer_list	timer;
>>   	unsigned long		used;
>>   	atomic_t		probes;
>> -	__u8			flags;
>> -	__u8			nud_state;
>> -	__u8			type;
>> -	__u8			dead;
>> +	u8			nud_state;
>> +	u8			type;
>> +	u8			dead;
>>   	u8			protocol;
>> +	u32			flags;
>>   	seqlock_t		ha_lock;
>>   	unsigned char		ha[ALIGN(MAX_ADDR_LEN, sizeof(unsigned long))] __aligned(8);
>>   	struct hh_cache		hh;
>> @@ -172,7 +172,7 @@ struct pneigh_entry {
>>   	struct pneigh_entry	*next;
>>   	possible_net_t		net;
>>   	struct net_device	*dev;
>> -	u8			flags;
>> +	u32			flags;
>>   	u8			protocol;
>>   	u8			key[];
>>   };
>> @@ -258,6 +258,10 @@ static inline void *neighbour_priv(const struct neighbour *n)
>>   #define NEIGH_UPDATE_F_ISROUTER			0x40000000
>>   #define NEIGH_UPDATE_F_ADMIN			0x80000000
>>   
>> +/* In-kernel representation for NDA_FLAGS_EXT flags: */
>> +#define NTF_OLD_MASK		0xff
>> +#define NTF_EXT_SHIFT		8
> 
> so only 24 EXT flags can be added. That should be documented; far off
> today, but that's an easy overflow to miss.

Agree, far off today, but this is only kernel internal, so there's always the
option to extend it iff really needed e.g. with u64 as neigh->flags. I'll add
a comment.

> Reviewed-by: David Ahern <dsahern@...nel.org>

Thanks!