Skip to content

Commit

Permalink
userns: fix id map generating with holes under specific conditions
Browse files Browse the repository at this point in the history
The uid/gid map generation would incorrectly leave holes when the
current ID was greater than 2^16-1 and a combination of other factors.

The gist of it is that bst, assuming the current UID is 93119, and the
following /etc/subuid:

    93119:100000:400000

... combined with the following system UID map:

    0       1000    1
    93119   93119   1
    1       100000  93118
    93120   193118  406882

... would need to attribute up to 400000 UIDs from the outer 100000 UID.

This attribution is done by mapping the current UID to inner UID 0, then
walking over the set of allotted subuids as defined in the /etc/subuid
file.

However, reading /etc/subuid would produce the following ID map:

    0       93119   1
    1       100000  400000

That is to say, bst would, upon reading /etc/subuid, insert an implicit
93119:93119:1 entry at the top of the file.

The problem is that the uid map generation would already insert this
implicit entry, and _then_ it would start assigning uids in the allotted
map loaded from /etc/subuid. Normally, the generation code ignores
completely any allotted range in [0, 65535), which means that for UIDs
lower than 2^16, the implicit entry in /etc/subuid is ignored, but in
this scenario, the UID is greater than 2^16, so it does not get ignored.

This means that the uid would get mapped twice: once to UID 0, and once
to uid 1:

    0       93119   1
    1       93119   1
    2       100000  400000

Now comes the crux of the problem: it's not possible to map two
different inner UIDs to the same outer UID, so projecting this map onto
the current uid space causes this map to be generated:

    0       93119   1
    2       100000  400000

That is to say, UID 1 would be left unmapped.

This commit fixes this bug by ignoring the current id from the allotted
map if encountered. We keep that implicit entry because it is used to
determine whether or not it is acceptable to map some arbitrary ID
ranges within the boundaries of the sub-id map, and the current UID/GID
is allowed to get mapped to any arbitrary inner UID/GID.

This commit also removes the logic that ignores the range [0-65535),
because it was broken, and was really only used to attempt to address
the aforementioned problem.
  • Loading branch information
Snaipe committed Dec 6, 2023
1 parent e757b7b commit fcca8e7
Showing 1 changed file with 15 additions and 8 deletions.
23 changes: 15 additions & 8 deletions userns.c
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,17 @@ void id_map_parse(id_map map, char *opt)
$ cat /etc/subuid
barney:100000:65535
barney:200000:100000
Then, calling the function as such:
id_map_load_subids(map, "/etc/subuid", 1000, "barney");
will populate the contents of `map` with the following data:
inner=0 outer=1000 1
inner=0 outer=1000 1
inner=0 outer=100000 65535
inner=0 outer=200000 100000
*/
void id_map_load_subids(id_map map, const char *subid_path, const struct id *id)
{
Expand Down Expand Up @@ -165,14 +167,19 @@ void id_map_generate(id_map allotted, id_map out, const char *subid_path, const
continue;
}

/* Ignore the [0-65535) range. */
if (range.outer <= ID_MAX) {
uint32_t shift = ID_MAX + 1 - range.outer;
if (shift > range.length) {
shift = range.length;
/* The current id already has been assigned; ignore it if it comes up
in the allotted map. */
if (range.outer <= id->id && id->id < range.outer + range.length) {
uint32_t len = id->id - range.outer;
if (len != 0) {
/* Add range up to (but excluding) id->id */
j = id_map_append(tmp, j, cur_id, range.outer, len);
cur_id += len;
}
range.outer += shift;
range.length -= shift;

/* Change the range to start one past id->id */
range.outer = id->id + 1;
range.length -= len + 1;
continue;
}

Expand Down

0 comments on commit fcca8e7

Please sign in to comment.