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
