Messages in this thread | | | Date | Thu, 11 Nov 2021 07:46:04 -0800 | From | Tadeusz Struk <> | Subject | Re: [PATCH] skbuff: suppress clang object-size-mismatch error |
| |
Hi Marco, On 11/11/21 01:51, Marco Elver wrote: > On Thu, 11 Nov 2021 at 01:36, Tadeusz Struk<tadeusz.struk@linaro.org> wrote: >> Kernel throws a runtime object-size-mismatch error in skbuff queue >> helpers like in [1]. This happens every time there is a pattern >> like the below: >> >> int skbuf_xmit(struct sk_buff *skb) >> { >> struct sk_buff_head list; >> >> __skb_queue_head_init(&list); >> __skb_queue_tail(&list, skb); <-- offending call >> >> return do_xmit(net, &list); >> } >> >> and the kernel is build with clang and -fsanitize=undefined flag set. >> The reason is that the functions __skb_queue_[tail|head]() access the >> struct sk_buff_head object via a pointer to struct sk_buff, which is >> much bigger in size than the sk_buff_head. This could cause undefined >> behavior and clang is complaining: >> >> UBSAN: object-size-mismatch in ./include/linux/skbuff.h:2023:28 >> member access within address ffffc90000cb71c0 with insufficient space >> for an object of type 'struct sk_buff' > The config includes CONFIG_UBSAN_OBJECT_SIZE, right? Normally that's > disabled by default, probably why nobody has noticed these much.
Right, in all the defconfigs CONFIG_UBSAN_OBJECT_SIZE is not set.
> >> Suppress the error with __attribute__((no_sanitize("undefined"))) >> in the skb helpers. > Isn't there a better way, because doing this might also suppress other > issues wholesale. __no_sanitize_undefined should be the last resort. >
The other way to fix it would be to make the struct sk_buff_head equal in size with struct sk_buff:
struct sk_buff_head { - /* These two members must be first. */ - struct sk_buff *next; - struct sk_buff *prev; + union { + struct { + /* These two members must be first. */ + struct sk_buff *next; + struct sk_buff *prev;
- __u32 qlen; - spinlock_t lock; + __u32 qlen; + spinlock_t lock; + }; + struct sk_buff __prv; + }; };
but that's much more invasive, and I don't even have means to quantify this in terms of final binary size and performance impact. I think that would be a flat out no go.
From the other hand if you look at the __skb_queue functions they don't do much and at all so there is no much room for other issues really. I followed the suggestion in [1]:
"if your function deliberately contains possible ..., you can use __attribute__((no_sanitize... "
[1] https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html
-- Thanks, Tadeusz
| |