Re: [PATCH 1/2 v4] user namespace: use union in {g,u}idmap struct

[Christian Brauner <christian.brauner@canonical.com>]

On Mon, Oct 23, 2017 at 09:45:34AM +0300, Nikolay Borisov wrote:
> 
> 
> On 23.10.2017 09:39, Nikolay Borisov wrote:
> > 
> > 
> > On 19.10.2017 22:11, Christian Brauner wrote:
> >> - Add a struct containing two pointer to extents and wrap both the static extent
> >>   array and the struct into a union. This is done in preparation for bumping the
> >>   {g,u}idmap limits for user namespaces.
> >> - Add brackets around anonymous union when using designated initializers to
> >>   initialize members in order to please gcc <= 4.4.
> >>
> >> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> >> ---
> >> Changelog 2017-10-19:
> >> * kernel/user.c: Use brackets around anonymous union when using designated
> >>   initializers to initialize members. This is done to please gcc <= 4.4.
> >> ---
> >>  include/linux/user_namespace.h | 18 +++++++++++++-----
> >>  kernel/user.c                  | 30 ++++++++++++++++++------------
> >>  2 files changed, 31 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> >> index c18e01252346..7c83d7f6289b 100644
> >> --- a/include/linux/user_namespace.h
> >> +++ b/include/linux/user_namespace.h
> >> @@ -12,13 +12,21 @@
> >>  
> >>  #define UID_GID_MAP_MAX_EXTENTS 5
> >>  
> >> +struct uid_gid_extent {
> >> +	u32 first;
> >> +	u32 lower_first;
> >> +	u32 count;
> >> +};
> >> +
> >>  struct uid_gid_map {	/* 64 bytes -- 1 cache line */
> >>  	u32 nr_extents;
> >> -	struct uid_gid_extent {
> >> -		u32 first;
> >> -		u32 lower_first;
> >> -		u32 count;
> >> -	} extent[UID_GID_MAP_MAX_EXTENTS];
> >> +	union {
> >> +		struct uid_gid_extent extent[UID_GID_MAP_MAX_EXTENTS];
> >> +		struct {
> >> +			struct uid_gid_extent *forward;
> >> +			struct uid_gid_extent *reverse;
> >> +		};
> >> +	};
> >>  };
> >>  
> >>  #define USERNS_SETGROUPS_ALLOWED 1UL
> >> diff --git a/kernel/user.c b/kernel/user.c
> >> index 00281add65b2..9a20acce460d 100644
> >> --- a/kernel/user.c
> >> +++ b/kernel/user.c
> >> @@ -26,26 +26,32 @@
> >>  struct user_namespace init_user_ns = {
> >>  	.uid_map = {
> >>  		.nr_extents = 1,
> >> -		.extent[0] = {
> >> -			.first = 0,
> >> -			.lower_first = 0,
> >> -			.count = 4294967295U,
> >> +		{
> >> +			.extent[0] = {
> >> +				.first = 0,
> >> +				.lower_first = 0,
> >> +				.count = 4294967295U,
> > 
> > nit: ULONG_MAX ?
> 
> Well, actually UINT_MAX

This is not actually code I need to touch. This patch really only exists for the
sake of preventing gcc <= 4.4 from complaining by placing the union inside of
additional brackets. The type for here is u32. Since this was added by Eric I'm
assuming - for now - that he had a reason to hardcode this number rather than
using UINT_MAX.