[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

Nuno Lopes via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 18 10:13:50 PST 2022


nlopes added a comment.

In D119816#3332414 <https://reviews.llvm.org/D119816#3332414>, @ztong0001 wrote:

> In D119816#3331658 <https://reviews.llvm.org/D119816#3331658>, @nlopes wrote:
>
>> Well, this patch is just a band-aid and a disaster waiting to happen.
>> If kmalloc is tagged with an `__attribute__` stating the allocation size, then you can't dereference beyond that limit or risk the alias analysis do something you don't want. You are triggering UB like that.
>> Can't you just remove the `__attribute__` from kmalloc instead to avoid triggering UB?
>
> I am not sure I fully get what you are saying.
> What I am suggesting is something like below to avoid bounds check pass from running on this function.
>
>   diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>   index 9d0388bed0c1..186fca35266d 100644
>   --- a/net/core/skbuff.c
>   +++ b/net/core/skbuff.c
>   @@ -1679,7 +1679,7 @@ EXPORT_SYMBOL(__pskb_copy_fclone);
>     *     All the pointers pointing into skb header may change and must be
>     *     reloaded after call to this function.
>     */
>   -
>   +__attribute__((no_sanitize("bounds")))
>    int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
>                        gfp_t gfp_mask)
>    {

The main issue is that the kernel is wrong. It has a bug. The sanitizer's error is not a false-positive!
So what you are proposing is a band-aid. It's not a real solution and it's just masking a wider problem. LLVM knows that kmalloc(x) allocates x bytes because someone placed an `__attribute__ ((alloc_size (1)))` on kmalloc. That attribute is just wrong and should be removed. It allows LLVM to mark all accesses beyond `kmalloc(x) + x - 1` as undefined behavior.

TL;DR: this patch is not the solution for your problems.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119816/new/

https://reviews.llvm.org/D119816



More information about the cfe-commits mailing list