diff options
| author | Bill Wendling <morbo@google.com> | 2026-09-08 22:27:29 +0000 |
|---|---|---|
| committer | Christian Brauner <brauner@kernel.org> | 2026-09-09 09:45:01 +0200 |
| commit | b31397fe3007a9fc636fcc34fcdc2b1e69d46804 (patch) | |
| tree | b28546aec0638d698916c37ec3c1e8edfb63d2f7 | |
| parent | 03c9d11bfda9cdb81d4979f30ddb0f3c4d5a2aae (diff) | |
| download | linux-next-b31397fe3007a9fc636fcc34fcdc2b1e69d46804.tar.gz linux-next-b31397fe3007a9fc636fcc34fcdc2b1e69d46804.zip | |
userns: Add __counted_by_ptr attribute to struct uid_gid_map
The compiler attribute __counted_by_ptr associates a pointer field of a
struct with a sibling field within the same struct that specifies the
element count of the allocated memory. This enables KASAN and fortified
bounds-checking to detect out-of-bounds accesses to the pointer field at
runtime.
We can add the __counted_by_ptr attribute to the 'forward' and 'reverse'
pointer fields of 'struct uid_gid_map', which are counted by
'nr_extents'. Since 'nr_extents' is defined in a sibling anonymous
struct inside an anonymous union, the nearest common non-anonymous
struct level is 'struct uid_gid_map' itself, which is supported by the
compiler.
However, doing so has runtime implications. In the original
implementation of insert_extent(), elements are written to
map->forward[map->nr_extents] before map->nr_extents is incremented:
if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS)
dest = &map->extent[map->nr_extents];
else
dest = &map->forward[map->nr_extents];
*dest = *extent;
map->nr_extents++;
At the time of writing to 'map->forward[map->nr_extents]',
map->nr_extents is still 5, but we are accessing index 5 (which is the
6th element). Under __counted_by_ptr(nr_extents), the compiler and
KASAN expect the accessed index to be strictly less than
map->nr_extents. Therefore, accessing index 5 when the count is 5
triggers an out-of-bounds panic/trap at runtime.
To resolve this, insert_extent() is refactored to increment
map->nr_extents first, and then use map->nr_extents - 1 as the index:
map->nr_extents++;
if (map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS)
dest = &map->extent[map->nr_extents - 1];
else
dest = &map->forward[map->nr_extents - 1];
*dest = *extent;
Signed-off-by: Bill Wendling <morbo@google.com>
Link: https://patch.msgid.link/20260908222734.3048684-1-morbo@google.com
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Reviewed-by: Bradley Morgan <brads@mainlining.org>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
| -rw-r--r-- | include/linux/user_namespace.h | 4 | ||||
| -rw-r--r-- | kernel/user_namespace.c | 12 |
2 files changed, 10 insertions, 6 deletions
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h index e38d9e60569f..2962256eddf7 100644 --- a/include/linux/user_namespace.h +++ b/include/linux/user_namespace.h @@ -29,8 +29,8 @@ struct uid_gid_map { /* 64 bytes -- 1 cache line */ u32 nr_extents; }; struct { - struct uid_gid_extent *forward; - struct uid_gid_extent *reverse; + struct uid_gid_extent *forward __counted_by_ptr(nr_extents); + struct uid_gid_extent *reverse __counted_by_ptr(nr_extents); }; }; }; diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c index e9e04ce167df..7c8316c4271f 100644 --- a/kernel/user_namespace.c +++ b/kernel/user_namespace.c @@ -806,13 +806,17 @@ static int insert_extent(struct uid_gid_map *map, struct uid_gid_extent *extent) map->reverse = NULL; } - if (map->nr_extents < UID_GID_MAP_MAX_BASE_EXTENTS) - dest = &map->extent[map->nr_extents]; + /* + * nr_extents must be updated before the extent and forward arrays are + * accessed, otherwise KSAN will assert an out-of-bounds error. + */ + map->nr_extents++; + if (map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS) + dest = &map->extent[map->nr_extents - 1]; else - dest = &map->forward[map->nr_extents]; + dest = &map->forward[map->nr_extents - 1]; *dest = *extent; - map->nr_extents++; return 0; } |
