Re: [PATCH 3/5] userns: Don't read extents twice in m_start

[Nikolay Borisov]

On  1.11.2017 01:48, Eric W. Biederman wrote:
> 
> This is important so reading /proc/<pid>/{uid_map,gid_map,projid_map} while
> the map is being written does not do strange things.
> 
> Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
> ---
>  kernel/user_namespace.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 563a2981d7c7..4f7e357ac1e2 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -683,11 +683,13 @@ static void *m_start(struct seq_file *seq, loff_t *ppos,
>  		     struct uid_gid_map *map)
>  {
>  	loff_t pos = *ppos;
> +	unsigned extents = map->nr_extents;
> +	smp_rmb();

Barriers need to be paired to work correctly as well as have explicit
comments describing the pairing as per kernel coding style. Checkpatch
will actually produce warning for that particular memory barrier.

>  
> -	if (pos >= map->nr_extents)
> +	if (pos >= extents)
>  		return NULL;
>  
> -	if (map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS)
> +	if (extents <= UID_GID_MAP_MAX_BASE_EXTENTS)
>  		return &map->extent[pos];
>  
>  	return &map->forward[pos];
>