KSPP / linux

Linux kernel source tree (Kernel Self Protection Project)
https://kernsec.org/wiki/index.php/Kernel_Self_Protection_Project
Other
81 stars 5 forks source link

Fix PTR_ALIGN() to not obfuscate __builtin_object_size() #191

Open kees opened 2 years ago

kees commented 2 years ago

The PTR_ALIGN() macro masks __builtin_object_size()'s ability to see the referenced object.

struct thing {
    int a;
    int c[4];
    int b;
};

int *ok = &instance->c[1];
int *bad = PTR_ALIGN(&instance->c[1], 4);

__builtin_object_size(ok, 1) is correctly 12. __built_object_size(bad, 1) is incorrectly -1.

Raverstern commented 2 years ago

Hi @kees,

I did some research on it, and it seems it is the casting of the pointer to unsigned long within PTR_ALIGN() that causes the problem. Once we cast the pointer to an arithmetic type and then do any operation on it, the compiler can't do __built_object_size() to it anymore.

int *bad = (int *)((unsigned long)&instance.c[1] + 1);
                // ^ cast to arithmetic and then ^ whatever op
int a = __builtin_object_size(bad, 1); // returns -1

int *good = (void *)&instance.c[1] + 1;
         // ^ cast to pointer type is ok
int b = __builtin_object_size(good, 1); // returns 11

However, it's unexpectedly hard to achieve alignment without doing masking (&) on the address value itself (since we can't do & with operands of pointer types), and I only thought of a brute-force solution that was really ugly... So I wonder if you have any idea about it. Thanks.

kees commented 2 years ago

Yeah, I can't make this work. It looks like as soon as the link back to the original type is lost, all the __bos logic gets dropped on the floor:

https://godbolt.org/z/joEEKce9P

arichardson commented 2 years ago

I was hoping the clang alignment builtins that I added to clang 10 (https://clang.llvm.org/docs/LanguageExtensions.html#alignment-builtins) would be helpful here, but it seems that LLVM's __bos lowering still can't look through the generated IR even when using those functions instead of a pointer->int->pointer conversion (https://godbolt.org/z/sTn38be86). That being said, it might be easier to add this logic when processing IR without inttoptr.