On Fri, Mar 21, 2025 at 9:41 PM Kees Cook <kees@xxxxxxxxxx> wrote: > If __builtin_is_lvalue() is available, use it with __is_lvalue(). There > is patch to Clang to provide this builtin now[1]. > > Link: https://github.com/kees/llvm-project/commits/builtin_is_lvalue/ [1] > Signed-off-by: Kees Cook <kees@xxxxxxxxxx> Before you land that LLVM patch, it might be a good idea to figure out how this interacts with the fun C quirk where you can have temporary rvalues which can contain array members to which you can technically create lvalues but must not write. As far as I understand, calling "kfree(getS().ptrs[0])" as in the following example would cause UB with your patch: ``` $ cat kees-kfree-test.c #include <stdlib.h> #define __is_lvalue(expr) __builtin_is_lvalue(expr) void __kfree(void *ptr); void BEFORE_SET_TO_NULL(); void AFTER_SET_TO_NULL(); static inline void kfree_and_null(void **ptr) { __kfree(*ptr); BEFORE_SET_TO_NULL(); *ptr = NULL; AFTER_SET_TO_NULL(); } #define __force_lvalue_expr(x) \ __builtin_choose_expr(__is_lvalue(x), x, (void *){ NULL }) #define __free_and_null(__how, x) \ ({ \ typeof(x) *__ptr = &(x); \ __how ## _and_null((void **)__ptr); \ }) #define __free_and_maybe_null(__how, x) \ __builtin_choose_expr(__is_lvalue(x), \ __free_and_null(__how, __force_lvalue_expr(x)), \ __kfree(x)) #define kfree(x) __free_and_maybe_null(kfree, x) struct S { void *ptrs[1]; }; struct S getS(void); int is_lvalue_test(void) { return __is_lvalue(getS().ptrs[0]); } void testfn2(void) { kfree(getS().ptrs[0]); } $ [...]/bin/clang-14 -c -o kees-kfree-test.o kees-kfree-test.c -O3 -Wall $ objdump -d -Mintel -r kees-kfree-test.o kees-kfree-test.o: file format elf64-x86-64 Disassembly of section .text: 0000000000000000 <is_lvalue_test>: 0: b8 01 00 00 00 mov eax,0x1 5: c3 ret 6: 66 2e 0f 1f 84 00 00 cs nop WORD PTR [rax+rax*1+0x0] d: 00 00 00 0000000000000010 <testfn2>: 10: 50 push rax 11: e8 00 00 00 00 call 16 <testfn2+0x6> 12: R_X86_64_PLT32 getS-0x4 16: 48 89 c7 mov rdi,rax 19: e8 00 00 00 00 call 1e <testfn2+0xe> 1a: R_X86_64_PLT32 __kfree-0x4 1e: 31 c0 xor eax,eax 20: e8 00 00 00 00 call 25 <testfn2+0x15> 21: R_X86_64_PLT32 BEFORE_SET_TO_NULL-0x4 25: 31 c0 xor eax,eax 27: 59 pop rcx 28: e9 00 00 00 00 jmp 2d <testfn2+0x1d> 29: R_X86_64_PLT32 AFTER_SET_TO_NULL-0x4 jannh@horn:~/test/kees-kfree$ ``` As far as I understand, this causes UB in C99 ("If an attempt is made to modify the result of a function call or to access it after the next sequence point, the behavior is undefined.") and in C11 ("A non-lvalue expression with structure or union type, where the structure or union contains a member with array type (including, recursively, members of all contained structures and unions) refers to an object with automatic storage duration and temporary lifetime. 36) Its lifetime begins when the expression is evaluated and its initial value is the value of the expression. Its lifetime ends when the evaluation of the containing full expression or full declarator ends. Any attempt to modify an object with temporary lifetime results in undefined behavior."). Basically, something like getS().ptrs[0] gives you something that is technically an lvalue but must not actually be written to, and ->isModifiableLvalue() does not catch that.
