Compile time fortify warning in Linux kernel after commit d77067d08a3f56d

[nathanchance]

After d77067d, there is a new warning in the Linux kernel from its compile time fortify protections (from include/linux/fortify-string.h) in fs/smb/client/cifsencrypt.c, which indicates that a condition that can be checked at compile time was triggered. However, it seems to be a false positive.

I've come up with a standalone reproducer from the kernel sources directly, which can be run in user space:

repro.c

#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>

typedef unsigned long	__kernel_ulong_t;
typedef __kernel_ulong_t __kernel_size_t;
typedef __kernel_size_t		size_t;

#define SIZE_MAX	(~(size_t)0)

#define __alloc_size(x, ...)			__attribute__((__alloc_size__(x, ## __VA_ARGS__))) __attribute__((__malloc__))
#define __cold					__attribute__((__cold__))
#define __compiletime_error(msg)		__attribute__((__error__(msg)))
#define __compiletime_warning(msg)		__attribute__((__warning__(msg)))
#define __diagnose_as(builtin...)		__attribute__((__diagnose_as_builtin__(builtin)))
#define __pass_dynamic_object_size(type)	__attribute__((__pass_dynamic_object_size__(type)))
#define __gnu_inline				__attribute__((__gnu_inline__))
#define __noreturn				__attribute__((__noreturn__))
#define __overloadable				__attribute__((__overloadable__))

#define __FORTIFY_INLINE extern __always_inline __gnu_inline __overloadable
#define __RENAME(x) __asm__(#x)

#define __struct_size(p)	__builtin_dynamic_object_size(p, 0)
#define __member_size(p)	__builtin_dynamic_object_size(p, 1)
#define POS			__pass_dynamic_object_size(1)
#define POS0			__pass_dynamic_object_size(0)

void fortify_panic(const char *name) __noreturn __cold;
void __write_overflow(void) __compiletime_error("detected write beyond size of object (1st parameter)");
void __write_overflow_field(size_t avail, size_t wanted) __compiletime_warning("detected write beyond size of field (1st parameter); maybe use struct_group()?");

#define __compiletime_lessthan(bounds, length)	(	\
	__builtin_constant_p((bounds) < (length)) &&	\
	(bounds) < (length)				\
)

#define __compiletime_strlen(p)					\
({								\
	char *__p = (char *)(p);				\
	size_t __ret = SIZE_MAX;				\
	const size_t __p_size = __member_size(p);		\
	if (__p_size != SIZE_MAX &&				\
	    __builtin_constant_p(*__p)) {			\
		size_t __p_len = __p_size - 1;			\
		if (__builtin_constant_p(__p[__p_len]) &&	\
		    __p[__p_len] == '\0')			\
			__ret = __builtin_strlen(__p);		\
	}							\
	__ret;							\
})

#define __is_constexpr(x) \
	(sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))

#define strlen(p)							\
	__builtin_choose_expr(__is_constexpr(__builtin_strlen(p)),	\
		__builtin_strlen(p), __fortify_strlen(p))

#define __underlying_memset	__builtin_memset
#define __underlying_strlen	__builtin_strlen

extern __kernel_size_t __real_strnlen(const char *, __kernel_size_t) __RENAME(strnlen);
/**
 * strnlen - Return bounded count of characters in a NUL-terminated string
 *
 * @p: pointer to NUL-terminated string to count.
 * @maxlen: maximum number of characters to count.
 *
 * Returns number of characters in @p (NOT including the final NUL), or
 * @maxlen, if no NUL has been found up to there.
 *
 */
__FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size_t maxlen)
{
	const size_t p_size = __member_size(p);
	const size_t p_len = __compiletime_strlen(p);
	size_t ret;

	/* We can take compile-time actions when maxlen is const. */
	if (__builtin_constant_p(maxlen) && p_len != SIZE_MAX) {
		/* If p is const, we can use its compile-time-known len. */
		if (maxlen >= p_size)
			return p_len;
	}

	/* Do not check characters beyond the end of p. */
	ret = __real_strnlen(p, maxlen < p_size ? maxlen : p_size);
	if (p_size <= ret && maxlen != ret)
		fortify_panic(__func__);
	return ret;
}

__FORTIFY_INLINE __diagnose_as(__builtin_strlen, 1)
__kernel_size_t __fortify_strlen(const char * const POS p)
{
	const size_t p_size = __member_size(p);
	__kernel_size_t ret;

	/* Give up if we don't know how large p is. */
	if (p_size == SIZE_MAX)
		return __underlying_strlen(p);
	ret = strnlen(p, p_size);
	if (p_size <= ret)
		fortify_panic(__func__);
	return ret;
}

__FORTIFY_INLINE void fortify_memset_chk(__kernel_size_t size,
					 const size_t p_size,
					 const size_t p_size_field)
{
	if (__builtin_constant_p(size)) {
		/*
		 * Length argument is a constant expression, so we
		 * can perform compile-time bounds checking where
		 * buffer sizes are also known at compile time.
		 */

		/* Error when size is larger than enclosing struct. */
		if (__compiletime_lessthan(p_size_field, p_size) &&
		    __compiletime_lessthan(p_size, size))
			__write_overflow();

		/* Warn when write size is larger than dest field. */
		if (__compiletime_lessthan(p_size_field, size))
			__write_overflow_field(p_size_field, size);
	}
	/*
	 * At this point, length argument may not be a constant expression,
	 * so run-time bounds checking can be done where buffer sizes are
	 * known. (This is not an "else" because the above checks may only
	 * be compile-time warnings, and we want to still warn for run-time
	 * overflows.)
	 */

	/*
	 * Always stop accesses beyond the struct that contains the
	 * field, when the buffer's remaining size is known.
	 * (The SIZE_MAX test is to optimize away checks where the buffer
	 * lengths are unknown.)
	 */
	if (p_size != SIZE_MAX && p_size < size)
		fortify_panic("memset");
}

#define __fortify_memset_chk(p, c, size, p_size, p_size_field) ({	\
	size_t __fortify_size = (size_t)(size);				\
	fortify_memset_chk(__fortify_size, p_size, p_size_field),	\
	__underlying_memset(p, c, __fortify_size);			\
})

#define memset(p, c, s) __fortify_memset_chk(p, c, s,			\
		__struct_size(p), __member_size(p))

#define show(exp) printf(#exp ": %zu\n", exp)

extern int crypto_shash_update(const unsigned char *buffer, unsigned int len);

static __always_inline __alloc_size(1) void *kmalloc(size_t size)
{
	return malloc(size);
}

int main(int argc, char **argv)
{
	unsigned short *user;
	char *user_name = NULL;
	int len;

	if (argc > 1)
		user_name = argv[1];

	len = user_name ? strlen(user_name) : 0;
	user = kmalloc(2 + (len * 2));
	if (!user)
		return -1;

	show(__struct_size(user));
	show(__member_size(user));

	if (len)
		printf("Length of user_name is not zero\n");
	else
		memset(user, '\0', 2);

	return crypto_shash_update((unsigned char *)user, len * 2);
}

lib.c

#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>

void fortify_panic(const char *name)
{
	fprintf(stderr, "detected buffer overflow in %s\n", name);
	exit(1);
}

size_t strnlen(const char *s, size_t count)
{
	const char *sc;

	for (sc = s; count-- && *sc != '\0'; ++sc)
		/* nothing */;
	return sc - s;
}

int crypto_shash_update(const unsigned char *buffer, unsigned int len)
{
	return len % 2;
}

void __write_overflow_field(size_t avail, size_t wanted)
{
}

$ clang -O2 repro.c lib.c
repro.c:127:4: warning: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribute-warning]
  127 |                         __write_overflow_field(p_size_field, size);
      |                         ^
1 warning generated.

$ ./a.out test
__struct_size(user): 10
__member_size(user): 10
Length of user_name is not zero

$ ./a.out
__struct_size(user): 2
__member_size(user): 2

In the else block, __builtin_dynamic_object_size() can only ever return 2 because we know len is zero (i.e., p_size_field == 2 due to __alloc_size(1) on kmalloc()) and the size to memset() is 2 (i.e., size == 2), so the condition p_size_field < size is always false, but it appears that after that change it is no longer eliminated (or sunk in some other manner?).

Some other potentially relevant information is in the downstream issue.

cc @nikic @nickdesaulniers @kees

[dtcxzyw]

Alive2: https://alive2.llvm.org/ce/z/GUuN4i

[nickdesaulniers]

cc @regehr