lkml.org 
[lkml]   [2021]   [Nov]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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
>

\
 
 \ /
  Last update: 2021-11-11 10:53    [W:0.072 / U:2.324 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site