[PATCH 1/2] userns: Add __counted_by_ptr attribute to struct uid_gid_map
Bill Wendling <[email protected]>
| Newsgroups | org.kernel.vger.linux-hardening,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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; Assisted-by: Gemini Next Signed-off-by: Bill Wendling <[email protected]> --- Cc: Kees Cook <[email protected]> Cc: "Gustavo A. R. Silva" <[email protected]> Cc: Christian Brauner <[email protected]> Cc: Aleksa Sarai <[email protected]> Cc: Jan Kara <[email protected]> Cc: Nathan Chancellor <[email protected]> Cc: Miguel Ojeda <[email protected]> Cc: Thomas Gleixner <[email protected]> Cc: Nicolas Schier <[email protected]> Cc: Gary Guo <[email protected]> Cc: "Thomas Weißschuh" <[email protected]> Cc: Alice Ryhl <[email protected]> Cc: Douglas Anderson <[email protected]> Cc: Anand Moon <[email protected]> Cc: Oleg Nesterov <[email protected]> Cc: [email protected] Cc: [email protected] Cc: [email protected] --- include/linux/user_namespace.h | 4 ++-- kernel/user_namespace.c | 8 ++++---- 2 files changed, 6 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 0bed462e9b2a..7e5371d8f515 100644 --- a/kernel/user_namespace.c +++ b/kernel/user_namespace.c @@ -809,13 +809,13 @@ 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]; + 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; } -- 2.55.0.860.g4b6b3295ed-goog