On 23 August 2026 13:51:47 BST, Bill Wendling <[email protected]> wrote: >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. >
Ack. >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. love it. >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: > Resounding ack. > 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. oh! >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 Id like to wonder what the hell that model is Gemini 3.5 pro?, I gave it a Google and saw nothing. Btw. Reviewed-by: Bradley Morgan <[email protected]> >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 <cyphar> @cyphar.com> >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; > } > > Thanks!
