netdev - Re: [PATCH net-next 2/3] netlink: add variable-length / auto integers

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: <62714d2c-8afc-4d9b-b8b2-85f9caf18eeb@6wind.com>
Date: Fri, 20 Oct 2023 09:24:34 +0200
From: Nicolas Dichtel <nicolas.dichtel@...nd.com>
To: Jakub Kicinski <kuba@...nel.org>, davem@...emloft.net
Cc: netdev@...r.kernel.org, edumazet@...gle.com, pabeni@...hat.com,
 johannes@...solutions.net, stephen@...workplumber.org, jiri@...nulli.us
Subject: Re: [PATCH net-next 2/3] netlink: add variable-length / auto integers

Le 18/10/2023 à 23:39, Jakub Kicinski a écrit :
> We currently push everyone to use padding to align 64b values
> in netlink. Un-padded nla_put_u64() doesn't even exist any more.
> 
> The story behind this possibly start with this thread:
> https://lore.kernel.org/netdev/20121204.130914.1457976839967676240.davem@davemloft.net/
> where DaveM was concerned about the alignment of a structure
> containing 64b stats. If user space tries to access such struct
> directly:
> 
> 	struct some_stats *stats = nla_data(attr);
> 	printf("A: %llu", stats->a);
> 
> lack of alignment may become problematic for some architectures.
> These days we most often put every single member in a separate
> attribute, meaning that the code above would use a helper like
> nla_get_u64(), which can deal with alignment internally.
> Even for arches which don't have good unaligned access - access
> aligned to 4B should be pretty efficient.
> Kernel and well known libraries deal with unaligned input already.
> 
> Padded 64b is quite space-inefficient (64b + pad means at worst 16B
> per attr vs 32b which takes 8B). It is also more typing:
> 
>     if (nla_put_u64_pad(rsp, NETDEV_A_SOMETHING_SOMETHING,
>                         value, NETDEV_A_SOMETHING_PAD))
> 
> Create a new attribute type which will use 32 bits at netlink
> level if value is small enough (probably most of the time?),
> and (4B-aligned) 64 bits otherwise. Kernel API is just:
> 
>     if (nla_put_uint(rsp, NETDEV_A_SOMETHING_SOMETHING, value))
> 
> Calling this new type "just" sint / uint with no specific size
> will hopefully also make people more comfortable with using it.
> Currently telling people "don't use u8, you may need the bits,
> and netlink will round up to 4B, anyway" is the #1 comment
> we give to newcomers.
> 
> In terms of netlink layout it looks like this:
> 
>          0       4       8       12      16
> 32b:     [nlattr][ u32  ]
> 64b:     [  pad ][nlattr][     u64      ]
> uint(32) [nlattr][ u32  ]
> uint(64) [nlattr][     u64      ]
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
Acked-by: Nicolas Dichtel <nicolas.dichtel@...nd.com>