Hi,
GCC and Clang both use the "alloc_size" attribute to assist with bounds
checking around the use of allocation functions. Add the attribute,
adjust the Makefile to silence needless warnings, and add the hints to
the allocators where possible. These changes have been in use for a
while now in GrapheneOS.
To build without warnings, this series needs a couple small fixes for
allmodconfig, which I sent separately:
https://lore.kernel.org/lkml/[email protected]/
https://lore.kernel.org/lkml/[email protected]/
https://lore.kernel.org/lkml/[email protected]/
I figure I can take this via my "overflow" series, or it could go via
-mm?
-Kees
Kees Cook (5):
Compiler Attributes: Add __alloc_size() for better bounds checking
slab: Add __alloc_size attributes for better bounds checking
mm/page_alloc: Add __alloc_size attributes for better bounds checking
percpu: Add __alloc_size attributes for better bounds checking
mm/vmalloc: Add __alloc_size attributes for better bounds checking
Makefile | 6 +++-
include/linux/compiler_attributes.h | 6 ++++
include/linux/gfp.h | 4 +--
include/linux/percpu.h | 6 ++--
include/linux/slab.h | 50 ++++++++++++++++++-----------
include/linux/vmalloc.h | 22 ++++++-------
6 files changed, 58 insertions(+), 36 deletions(-)
--
2.30.2
As already done in GrapheneOS, add the __alloc_size attribute for
appropriate vmalloc allocator interfaces, to provide additional hinting
for better bounds checking, assisting CONFIG_FORTIFY_SOURCE and other
compiler optimizations.
Co-developed-by: Daniel Micay <[email protected]>
Signed-off-by: Daniel Micay <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: [email protected]
Signed-off-by: Kees Cook <[email protected]>
---
include/linux/vmalloc.h | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 2644425b6dce..f4ede07e1dae 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -136,21 +136,21 @@ static inline void vmalloc_init(void)
static inline unsigned long vmalloc_nr_pages(void) { return 0; }
#endif
-extern void *vmalloc(unsigned long size);
-extern void *vzalloc(unsigned long size);
-extern void *vmalloc_user(unsigned long size);
-extern void *vmalloc_node(unsigned long size, int node);
-extern void *vzalloc_node(unsigned long size, int node);
-extern void *vmalloc_32(unsigned long size);
-extern void *vmalloc_32_user(unsigned long size);
-extern void *__vmalloc(unsigned long size, gfp_t gfp_mask);
+extern void *vmalloc(unsigned long size) __alloc_size(1);
+extern void *vzalloc(unsigned long size) __alloc_size(1);
+extern void *vmalloc_user(unsigned long size) __alloc_size(1);
+extern void *vmalloc_node(unsigned long size, int node) __alloc_size(1);
+extern void *vzalloc_node(unsigned long size, int node) __alloc_size(1);
+extern void *vmalloc_32(unsigned long size) __alloc_size(1);
+extern void *vmalloc_32_user(unsigned long size) __alloc_size(1);
+extern void *__vmalloc(unsigned long size, gfp_t gfp_mask) __alloc_size(1);
extern void *__vmalloc_node_range(unsigned long size, unsigned long align,
unsigned long start, unsigned long end, gfp_t gfp_mask,
pgprot_t prot, unsigned long vm_flags, int node,
- const void *caller);
+ const void *caller) __alloc_size(1);
void *__vmalloc_node(unsigned long size, unsigned long align, gfp_t gfp_mask,
- int node, const void *caller);
-void *vmalloc_no_huge(unsigned long size);
+ int node, const void *caller) __alloc_size(1);
+void *vmalloc_no_huge(unsigned long size) __alloc_size(1);
extern void vfree(const void *addr);
extern void vfree_atomic(const void *addr);
--
2.30.2
On Tue, Aug 17, 2021 at 10:08:36PM -0700, Kees Cook wrote:
> Hi,
>
> GCC and Clang both use the "alloc_size" attribute to assist with bounds
> checking around the use of allocation functions. Add the attribute,
> adjust the Makefile to silence needless warnings, and add the hints to
> the allocators where possible. These changes have been in use for a
> while now in GrapheneOS.
Can you explain how this attribute helps? Should we flow it through
other allocating functions?
It tells the compiler the function will either return NULL or an
allocation of the size specific by the parameter referenced by
alloc_size. It could also be used for functions resembling allocation
functions which aren't actually allocating. The compiler will use it
for optimization so it's extremely important that it's only used
correctly. It only really has a use on the top-level API used
externally.
The compiler uses it for __builtin_object_size which is primarily used
by FORTIFY_SOURCE and also internally by -fsanitize=object-size which
will be available for the kernel via UBSan to find bugs or as
hardening in the trapping mode. There are currently compatibility
issues (undefined out-of-bounds accesses) blocking using
-fsanitize=object-size beyond fixing those relatively benign issues to
allow using it elsewhere.
For example, it will know that kmalloc(n) returns either NULL or an
allocation of size n. A simple sample program with calloc in
userspace:
#include <stdlib.h>
#include <stdio.h>
int main(void) {
char *p = calloc(64, 1);
if (!p) {
return 1;
}
printf("%zu\n", __builtin_object_size(p, 1));
return 0;
}
It will also detect an out-of-bounds access via the allocation with
-fsanitize=object-size including with a runtime value as the index.
It's not as useful as it should be yet because __builtin_object_size
must return a compile-time constant. Clang has a new
__builtin_dynamic_object_size that's allowed to return a value that's
not a compile-time constant so it can work for kmalloc(n) where n is a
runtime value. It might not be quite ready for use yet but it should
be able to make it a lot more useful. GCC also seems open to adding it
too.
On Thu, 19 Aug 2021, Daniel Micay wrote:
> For example, it will know that kmalloc(n) returns either NULL or an
> allocation of size n. A simple sample program with calloc in
> userspace:
>
> #include <stdlib.h>
> #include <stdio.h>
>
> int main(void) {
> char *p = calloc(64, 1);
> if (!p) {
> return 1;
> }
> printf("%zu\n", __builtin_object_size(p, 1));
> return 0;
> }
>
> It will also detect an out-of-bounds access via the allocation with
> -fsanitize=object-size including with a runtime value as the index.
>
> It's not as useful as it should be yet because __builtin_object_size
> must return a compile-time constant. Clang has a new
> __builtin_dynamic_object_size that's allowed to return a value that's
> not a compile-time constant so it can work for kmalloc(n) where n is a
> runtime value. It might not be quite ready for use yet but it should
> be able to make it a lot more useful. GCC also seems open to adding it
> too.
The other complication with kmalloc etc is that the slab allocators may
decided to allocate more bytes than needed because it does not support
that particular allocation size. Some functions check the allocated true
size and make use of that. See ksize().
On Wed, Aug 25, 2021 at 12:01:42PM +0200, Christoph Lameter wrote:
> On Thu, 19 Aug 2021, Daniel Micay wrote:
>
> > For example, it will know that kmalloc(n) returns either NULL or an
> > allocation of size n. A simple sample program with calloc in
> > userspace:
> >
> > #include <stdlib.h>
> > #include <stdio.h>
> >
> > int main(void) {
> > char *p = calloc(64, 1);
> > if (!p) {
> > return 1;
> > }
> > printf("%zu\n", __builtin_object_size(p, 1));
> > return 0;
> > }
> >
> > It will also detect an out-of-bounds access via the allocation with
> > -fsanitize=object-size including with a runtime value as the index.
> >
> > It's not as useful as it should be yet because __builtin_object_size
> > must return a compile-time constant. Clang has a new
> > __builtin_dynamic_object_size that's allowed to return a value that's
> > not a compile-time constant so it can work for kmalloc(n) where n is a
> > runtime value. It might not be quite ready for use yet but it should
> > be able to make it a lot more useful. GCC also seems open to adding it
> > too.
>
> The other complication with kmalloc etc is that the slab allocators may
> decided to allocate more bytes than needed because it does not support
> that particular allocation size. Some functions check the allocated true
> size and make use of that. See ksize().
Yup, this is known. For the current iteration, this doesn't pose a
problem since the compile-time checking has very limited scope.
--
Kees Cook