Messages in this thread | | | From | Marco Elver <> | Date | Thu, 11 Nov 2021 10:51:41 +0100 | Subject | Re: [PATCH] skbuff: suppress clang object-size-mismatch error |
| |
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.
> 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.
> [1] https://syzkaller.appspot.com/bug?id=5d9f0bca58cea80f272b73500df67dcd9e35c886 > > Cc: "Nathan Chancellor" <nathan@kernel.org> > Cc: "Nick Desaulniers" <ndesaulniers@google.com> > Cc: "Jakub Kicinski" <kuba@kernel.org> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: "Jonathan Lemon" <jonathan.lemon@gmail.com> > Cc: "Alexander Lobakin" <alobakin@pm.me> > Cc: "Willem de Bruijn" <willemb@google.com> > Cc: "Paolo Abeni" <pabeni@redhat.com> > Cc: "Cong Wang" <cong.wang@bytedance.com> > Cc: "Kevin Hao" <haokexin@gmail.com> > Cc: "Ilias Apalodimas" <ilias.apalodimas@linaro.org> > Cc: "Marco Elver" <elver@google.com> > Cc: <netdev@vger.kernel.org> > Cc: <linux-kernel@vger.kernel.org> > Cc: <llvm@lists.linux.dev> > > Signed-off-by: Tadeusz Struk <tadeusz.struk@linaro.org> > --- > include/linux/skbuff.h | 49 ++++++++++++++++++++++++------------------ > 1 file changed, 28 insertions(+), 21 deletions(-) > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index 0bd6520329f6..8ec46e3a503d 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h > @@ -1933,9 +1933,10 @@ static inline void skb_queue_head_init_class(struct sk_buff_head *list, > * The "__skb_xxxx()" functions are the non-atomic ones that > * can only be called with interrupts disabled. > */ > -static inline void __skb_insert(struct sk_buff *newsk, > - struct sk_buff *prev, struct sk_buff *next, > - struct sk_buff_head *list) > +static inline void __no_sanitize_undefined > +__skb_insert(struct sk_buff *newsk, > + struct sk_buff *prev, struct sk_buff *next, > + struct sk_buff_head *list) > { > /* See skb_queue_empty_lockless() and skb_peek_tail() > * for the opposite READ_ONCE() > @@ -1966,8 +1967,9 @@ static inline void __skb_queue_splice(const struct sk_buff_head *list, > * @list: the new list to add > * @head: the place to add it in the first list > */ > -static inline void skb_queue_splice(const struct sk_buff_head *list, > - struct sk_buff_head *head) > +static inline void __no_sanitize_undefined > +skb_queue_splice(const struct sk_buff_head *list, > + struct sk_buff_head *head) > { > if (!skb_queue_empty(list)) { > __skb_queue_splice(list, (struct sk_buff *) head, head->next); > @@ -1982,8 +1984,9 @@ static inline void skb_queue_splice(const struct sk_buff_head *list, > * > * The list at @list is reinitialised > */ > -static inline void skb_queue_splice_init(struct sk_buff_head *list, > - struct sk_buff_head *head) > +static inline void __no_sanitize_undefined > +skb_queue_splice_init(struct sk_buff_head *list, > + struct sk_buff_head *head) > { > if (!skb_queue_empty(list)) { > __skb_queue_splice(list, (struct sk_buff *) head, head->next); > @@ -1997,8 +2000,9 @@ static inline void skb_queue_splice_init(struct sk_buff_head *list, > * @list: the new list to add > * @head: the place to add it in the first list > */ > -static inline void skb_queue_splice_tail(const struct sk_buff_head *list, > - struct sk_buff_head *head) > +static inline void __no_sanitize_undefined > +skb_queue_splice_tail(const struct sk_buff_head *list, > + struct sk_buff_head *head) > { > if (!skb_queue_empty(list)) { > __skb_queue_splice(list, head->prev, (struct sk_buff *) head); > @@ -2014,8 +2018,9 @@ static inline void skb_queue_splice_tail(const struct sk_buff_head *list, > * Each of the lists is a queue. > * The list at @list is reinitialised > */ > -static inline void skb_queue_splice_tail_init(struct sk_buff_head *list, > - struct sk_buff_head *head) > +static inline void __no_sanitize_undefined > +skb_queue_splice_tail_init(struct sk_buff_head *list, > + struct sk_buff_head *head) > { > if (!skb_queue_empty(list)) { > __skb_queue_splice(list, head->prev, (struct sk_buff *) head); > @@ -2035,9 +2040,10 @@ static inline void skb_queue_splice_tail_init(struct sk_buff_head *list, > * > * A buffer cannot be placed on two lists at the same time. > */ > -static inline void __skb_queue_after(struct sk_buff_head *list, > - struct sk_buff *prev, > - struct sk_buff *newsk) > +static inline void __no_sanitize_undefined > +__skb_queue_after(struct sk_buff_head *list, > + struct sk_buff *prev, > + struct sk_buff *newsk) > { > __skb_insert(newsk, prev, prev->next, list); > } > @@ -2045,9 +2051,10 @@ static inline void __skb_queue_after(struct sk_buff_head *list, > void skb_append(struct sk_buff *old, struct sk_buff *newsk, > struct sk_buff_head *list); > > -static inline void __skb_queue_before(struct sk_buff_head *list, > - struct sk_buff *next, > - struct sk_buff *newsk) > +static inline void __no_sanitize_undefined > +__skb_queue_before(struct sk_buff_head *list, > + struct sk_buff *next, > + struct sk_buff *newsk) > { > __skb_insert(newsk, next->prev, next, list); > } > @@ -2062,8 +2069,8 @@ static inline void __skb_queue_before(struct sk_buff_head *list, > * > * A buffer cannot be placed on two lists at the same time. > */ > -static inline void __skb_queue_head(struct sk_buff_head *list, > - struct sk_buff *newsk) > +static inline void __no_sanitize_undefined > +__skb_queue_head(struct sk_buff_head *list, struct sk_buff *newsk) > { > __skb_queue_after(list, (struct sk_buff *)list, newsk); > } > @@ -2079,8 +2086,8 @@ void skb_queue_head(struct sk_buff_head *list, struct sk_buff *newsk); > * > * A buffer cannot be placed on two lists at the same time. > */ > -static inline void __skb_queue_tail(struct sk_buff_head *list, > - struct sk_buff *newsk) > +static inline void __no_sanitize_undefined > +__skb_queue_tail(struct sk_buff_head *list, struct sk_buff *newsk) > { > __skb_queue_before(list, (struct sk_buff *)list, newsk); > } > -- > 2.33.1 >
| |