From fcca8e76829c7fd497a3eb9e54fd74d06e020aec Mon Sep 17 00:00:00 2001 From: "Franklin \"Snaipe\" Mathieu" Date: Wed, 22 Nov 2023 17:56:29 +0100 Subject: [PATCH] userns: fix id map generating with holes under specific conditions 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. --- userns.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/userns.c b/userns.c index 37aeaf8..f5cf17b 100644 --- a/userns.c +++ b/userns.c @@ -90,6 +90,7 @@ void id_map_parse(id_map map, char *opt) $ cat /etc/subuid barney:100000:65535 + barney:200000:100000 Then, calling the function as such: @@ -97,8 +98,9 @@ void id_map_parse(id_map map, char *opt) 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) { @@ -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; }