| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 11 Nov 2021 10:51:41 +0100
From: Marco Elver <elver@...gle.com>
To: Tadeusz Struk <tadeusz.struk@...aro.org>
Cc: "David S. Miller" <davem@...emloft.net>,
Nathan Chancellor <nathan@...nel.org>,
Nick Desaulniers <ndesaulniers@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Jonathan Lemon <jonathan.lemon@...il.com>,
Alexander Lobakin <alobakin@...me>,
Willem de Bruijn <willemb@...gle.com>,
Paolo Abeni <pabeni@...hat.com>,
Cong Wang <cong.wang@...edance.com>,
Kevin Hao <haokexin@...il.com>,
Ilias Apalodimas <ilias.apalodimas@...aro.org>,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
llvm@...ts.linux.dev, Kees Cook <keescook@...omium.org>,
Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH] skbuff: suppress clang object-size-mismatch error
On Thu, 11 Nov 2021 at 01:36, Tadeusz Struk <tadeusz.struk@...aro.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@...nel.org>
> Cc: "Nick Desaulniers" <ndesaulniers@...gle.com>
> Cc: "Jakub Kicinski" <kuba@...nel.org>
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: "Jonathan Lemon" <jonathan.lemon@...il.com>
> Cc: "Alexander Lobakin" <alobakin@...me>
> Cc: "Willem de Bruijn" <willemb@...gle.com>
> Cc: "Paolo Abeni" <pabeni@...hat.com>
> Cc: "Cong Wang" <cong.wang@...edance.com>
> Cc: "Kevin Hao" <haokexin@...il.com>
> Cc: "Ilias Apalodimas" <ilias.apalodimas@...aro.org>
> Cc: "Marco Elver" <elver@...gle.com>
> Cc: <netdev@...r.kernel.org>
> Cc: <linux-kernel@...r.kernel.org>
> Cc: <llvm@...ts.linux.dev>
>
> Signed-off-by: Tadeusz Struk <tadeusz.struk@...aro.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
>
Powered by blists - more mailing lists