Re: [PATCH v5 1/2] userns: Add __counted_by_ptr attribute to struct uid_gid_map

[Oleg Nesterov <oleg@redhat.com>]

On 09/08, Bill Wendling wrote:
>
> We can add the __counted_by_ptr attribute to the 'forward' and 'reverse'
> pointer fields of 'struct uid_gid_map', which are counted by
> 'nr_extents'.

Since I am CC'ed again... (thanks btw)

Personally, I am not a huge fan of __counted_by_ptr, at least in this
particular case. But 1/2 looks technically correct, I think it doesn't
hurt ;)

So, FWIW

Reviewed-by: Oleg Nesterov <oleg@redhat.com>


> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -809,13 +809,17 @@ static int insert_extent(struct uid_gid_map *map, struct uid_gid_extent *extent)
>  		map->reverse = NULL;
>  	}
>
> -	if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS)
> -		dest = &map->extent[map->nr_extents];
> +	/*
> +	 * nr_extents must be updated before the extent and forward arrays are
> +	 * accessed, otherwise KSAN will assert an out-of-bounds error.
> +	 */
> +	map->nr_extents++;

Yes, but perhaps the comment should be more explicit...
I mean, something like

	... otherwise KSAN ... because of __counted_by_ptr(nr_extents)

Nevermind, this is just a minor/subjective nit.

Oleg.