Message ID | 1542094958-9482-2-git-send-email-tyhicks@canonical.com |
---|---|
State | New |
Headers | show |
Series | CVE-2018-18955 - Privilege escalation within a nested user namespace | expand |
On 13/11/2018 07:42, Tyler Hicks wrote: > From: Jann Horn <jannh@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@vger.kernel.org > Signed-off-by: Jann Horn <jannh@google.com> > Tested-by: Eric W. Biederman <ebiederm@xmission.com> > Reviewed-by: Eric W. Biederman <ebiederm@xmission.com> > Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> > > CVE-2018-18955 > > (cherry picked from commit d2f007dbe7e4c9583eea6eb04d60001e85c6f1bd) > Signed-off-by: Tyler Hicks <tyhicks@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@canonical.com>
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,