Re: [PATCH 1/2] userns: Add __counted_by_ptr attribute to struct uid_gid_map

[Bradley Morgan]

On 23 August 2026 13:51:47 BST, Bill Wendling <morbo@xxxxxxxxxx> 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 <include@xxxxxxxxx>




>Signed-off-by: Bill Wendling <morbo@xxxxxxxxxx>
>---
>Cc: Kees Cook <kees@xxxxxxxxxx>
>Cc: "Gustavo A. R. Silva" <gustavoars@xxxxxxxxxx>
>Cc: Christian Brauner <brauner@xxxxxxxxxx>
>Cc: Aleksa Sarai <cyphar>
@cyphar.com>
>Cc: Jan Kara <jack@xxxxxxx>
>Cc: Nathan Chancellor <nathan@xxxxxxxxxx>
>Cc: Miguel Ojeda <ojeda@xxxxxxxxxx>
>Cc: Thomas Gleixner <tglx@xxxxxxxxxx>
>Cc: Nicolas Schier <nsc@xxxxxxxxxx>
>Cc: Gary Guo <gary@xxxxxxxxxxx>
>Cc: "Thomas Weißschuh" <thomas.weissschuh@xxxxxxxxxxxxx>
>Cc: Alice Ryhl <aliceryhl@xxxxxxxxxx>
>Cc: Douglas Anderson <dianders@xxxxxxxxxxxx>
>Cc: Anand Moon <linux.amoon@xxxxxxxxx>
>Cc: Oleg Nesterov <oleg@xxxxxxxxxx>
>Cc: codemender-patching+linux@xxxxxxxxxx
>Cc: linux-kernel@xxxxxxxxxxxxxxx
>Cc: linux-hardening@xxxxxxxxxxxxxxx
>---
> 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!