ACK: [PATCH 1/1][B/C/D] userns: also map extents in the reverse map to kernel IDs

[Colin Ian King]

On 13/11/2018 07:42, Tyler Hicks wrote:
> From: Jann Horn <jannh at google.com>
> 
> BugLink: https://launchpad.net/bugs/1801924
> 
> The current logic first clones the extent array and sorts both copies, then
> maps the lower IDs of the forward mapping into the lower namespace, but
> doesn't map the lower IDs of the reverse mapping.
> 
> This means that code in a nested user namespace with >5 extents will see
> incorrect IDs. It also breaks some access checks, like
> inode_owner_or_capable() and privileged_wrt_inode_uidgid(), so a process
> can incorrectly appear to be capable relative to an inode.
> 
> To fix it, we have to make sure that the "lower_first" members of extents
> in both arrays are translated; and we have to make sure that the reverse
> map is sorted *after* the translation (since otherwise the translation can
> break the sorting).
> 
> This is CVE-2018-18955.
> 
> Fixes: 6397fac4915a ("userns: bump idmap limits to 340")
> Cc: stable at vger.kernel.org
> Signed-off-by: Jann Horn <jannh at google.com>
> Tested-by: Eric W. Biederman <ebiederm at xmission.com>
> Reviewed-by: Eric W. Biederman <ebiederm at xmission.com>
> Signed-off-by: Eric W. Biederman <ebiederm at xmission.com>
> 
> CVE-2018-18955
> 
> (cherry picked from commit d2f007dbe7e4c9583eea6eb04d60001e85c6f1bd)
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
> ---
>  kernel/user_namespace.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 840ab1fbbc3b..80ed67b1acd2 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -980,10 +980,6 @@ static ssize_t map_write(struct file *file, const char __user *buf,
>  	if (!new_idmap_permitted(file, ns, cap_setid, &new_map))
>  		goto out;
>  
> -	ret = sort_idmaps(&new_map);
> -	if (ret < 0)
> -		goto out;
> -
>  	ret = -EPERM;
>  	/* Map the lower ids from the parent user namespace to the
>  	 * kernel global id space.
> @@ -1010,6 +1006,14 @@ static ssize_t map_write(struct file *file, const char __user *buf,
>  		e->lower_first = lower_first;
>  	}
>  
> +	/*
> +	 * If we want to use binary search for lookup, this clones the extent
> +	 * array and sorts both copies.
> +	 */
> +	ret = sort_idmaps(&new_map);
> +	if (ret < 0)
> +		goto out;
> +
>  	/* Install the map */
>  	if (new_map.nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS) {
>  		memcpy(map->extent, new_map.extent,
> 

Clean cherry pick, looks sane to me.

Acked-by: Colin Ian King <colin.king at canonical.com>