* [PATCH] skbuff: suppress clang object-size-mismatch error
@ 2021-11-11 0:35 Tadeusz Struk
2021-11-11 9:51 ` Marco Elver
0 siblings, 1 reply; 10+ messages in thread
From: Tadeusz Struk @ 2021-11-11 0:35 UTC (permalink / raw)
To: David S. Miller
Cc: Tadeusz Struk, Nathan Chancellor, Nick Desaulniers,
Jakub Kicinski, Jonathan Lemon, Alexander Lobakin,
Willem de Bruijn, Paolo Abeni, Cong Wang, Kevin Hao,
Ilias Apalodimas, Marco Elver, netdev, linux-kernel, llvm
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'
Suppress the error with __attribute__((no_sanitize("undefined")))
in the skb helpers.
[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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] skbuff: suppress clang object-size-mismatch error
2021-11-11 0:35 [PATCH] skbuff: suppress clang object-size-mismatch error Tadeusz Struk
@ 2021-11-11 9:51 ` Marco Elver
2021-11-11 15:46 ` Tadeusz Struk
0 siblings, 1 reply; 10+ messages in thread
From: Marco Elver @ 2021-11-11 9:51 UTC (permalink / raw)
To: Tadeusz Struk
Cc: David S. Miller, Nathan Chancellor, Nick Desaulniers,
Jakub Kicinski, Jonathan Lemon, Alexander Lobakin,
Willem de Bruijn, Paolo Abeni, Cong Wang, Kevin Hao,
Ilias Apalodimas, netdev, linux-kernel, llvm, Kees Cook,
Eric Dumazet
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
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] skbuff: suppress clang object-size-mismatch error
2021-11-11 9:51 ` Marco Elver
@ 2021-11-11 15:46 ` Tadeusz Struk
2021-11-11 15:52 ` Marco Elver
0 siblings, 1 reply; 10+ messages in thread
From: Tadeusz Struk @ 2021-11-11 15:46 UTC (permalink / raw)
To: Marco Elver
Cc: David S. Miller, Nathan Chancellor, Nick Desaulniers,
Jakub Kicinski, Jonathan Lemon, Alexander Lobakin,
Willem de Bruijn, Paolo Abeni, Cong Wang, Kevin Hao,
Ilias Apalodimas, netdev, linux-kernel, llvm, Kees Cook,
Eric Dumazet
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] skbuff: suppress clang object-size-mismatch error
2021-11-11 15:46 ` Tadeusz Struk
@ 2021-11-11 15:52 ` Marco Elver
2021-11-11 16:01 ` Tadeusz Struk
0 siblings, 1 reply; 10+ messages in thread
From: Marco Elver @ 2021-11-11 15:52 UTC (permalink / raw)
To: Tadeusz Struk
Cc: David S. Miller, Nathan Chancellor, Nick Desaulniers,
Jakub Kicinski, Jonathan Lemon, Alexander Lobakin,
Willem de Bruijn, Paolo Abeni, Cong Wang, Kevin Hao,
Ilias Apalodimas, netdev, linux-kernel, llvm, Kees Cook,
Eric Dumazet
On Thu, 11 Nov 2021 at 16:46, Tadeusz Struk <tadeusz.struk@linaro.org> wrote:
>
> 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... "
That general advice might not be compatible with what the kernel
wants, especially since UBSAN_OBJECT_SIZE is normally disabled and I
think known to cause these issues in the kernel.
I'll defer to maintainers to decide what would be the preferred way of
handling this.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] skbuff: suppress clang object-size-mismatch error
2021-11-11 15:52 ` Marco Elver
@ 2021-11-11 16:01 ` Tadeusz Struk
2021-11-11 17:54 ` Jakub Kicinski
0 siblings, 1 reply; 10+ messages in thread
From: Tadeusz Struk @ 2021-11-11 16:01 UTC (permalink / raw)
To: Marco Elver
Cc: David S. Miller, Nathan Chancellor, Nick Desaulniers,
Jakub Kicinski, Jonathan Lemon, Alexander Lobakin,
Willem de Bruijn, Paolo Abeni, Cong Wang, Kevin Hao,
Ilias Apalodimas, netdev, linux-kernel, llvm, Kees Cook,
Eric Dumazet
On 11/11/21 07:52, Marco Elver wrote:
>> 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... "
> That general advice might not be compatible with what the kernel
> wants, especially since UBSAN_OBJECT_SIZE is normally disabled and I
> think known to cause these issues in the kernel.
>
> I'll defer to maintainers to decide what would be the preferred way of
> handling this.
Sure, I would also like to know if there is a better way of fixing this.
Thanks for your feedback.
--
Thanks,
Tadeusz
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] skbuff: suppress clang object-size-mismatch error
2021-11-11 16:01 ` Tadeusz Struk
@ 2021-11-11 17:54 ` Jakub Kicinski
2021-11-12 15:42 ` David Miller
0 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2021-11-11 17:54 UTC (permalink / raw)
To: Tadeusz Struk, David S. Miller
Cc: Marco Elver, Nathan Chancellor, Nick Desaulniers, Jonathan Lemon,
Alexander Lobakin, Willem de Bruijn, Paolo Abeni, Cong Wang,
Kevin Hao, Ilias Apalodimas, netdev, linux-kernel, llvm,
Kees Cook, Eric Dumazet
On Thu, 11 Nov 2021 08:01:26 -0800 Tadeusz Struk wrote:
> > That general advice might not be compatible with what the kernel
> > wants, especially since UBSAN_OBJECT_SIZE is normally disabled and I
> > think known to cause these issues in the kernel.
> >
> > I'll defer to maintainers to decide what would be the preferred way of
> > handling this.
>
> Sure, I would also like to know if there is a better way of fixing this.
> Thanks for your feedback.
I remember Dave was working thru the tree at some point to clean up all
skb->next/skb->prev accesses so that we can switch over to using normal
list helpers.
I'm not sure if that stalled due to lack of time or some fundamental
problems.
Seems like finishing that would let us clean up such misuses?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] skbuff: suppress clang object-size-mismatch error
2021-11-11 17:54 ` Jakub Kicinski
@ 2021-11-12 15:42 ` David Miller
2021-11-18 16:05 ` Tadeusz Struk
0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2021-11-12 15:42 UTC (permalink / raw)
To: kuba
Cc: tadeusz.struk, elver, nathan, ndesaulniers, jonathan.lemon,
alobakin, willemb, pabeni, cong.wang, haokexin, ilias.apalodimas,
netdev, linux-kernel, llvm, keescook, edumazet
From: Jakub Kicinski <kuba@kernel.org>
Date: Thu, 11 Nov 2021 09:54:44 -0800
> I'm not sure if that stalled due to lack of time or some fundamental
> problems.
ran out of time, then had a stroke...
> Seems like finishing that would let us clean up such misuses?
yes it would
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] skbuff: suppress clang object-size-mismatch error
2021-11-12 15:42 ` David Miller
@ 2021-11-18 16:05 ` Tadeusz Struk
2021-11-18 16:38 ` Jakub Kicinski
0 siblings, 1 reply; 10+ messages in thread
From: Tadeusz Struk @ 2021-11-18 16:05 UTC (permalink / raw)
To: David Miller, kuba
Cc: elver, nathan, ndesaulniers, jonathan.lemon, alobakin, willemb,
pabeni, cong.wang, haokexin, ilias.apalodimas, netdev,
linux-kernel, llvm, keescook, edumazet
On 11/12/21 07:42, David Miller wrote:
> From: Jakub Kicinski <kuba@kernel.org>
> Date: Thu, 11 Nov 2021 09:54:44 -0800
>
>> I'm not sure if that stalled due to lack of time or some fundamental
>> problems.
>
> ran out of time, then had a stroke...
>
>> Seems like finishing that would let us clean up such misuses?
>
> yes it would
>
so since there is not better way of suppressing the issue atm are
you ok with taking this fix for now?
--
Thanks,
Tadeusz
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] skbuff: suppress clang object-size-mismatch error
2021-11-18 16:05 ` Tadeusz Struk
@ 2021-11-18 16:38 ` Jakub Kicinski
2021-11-18 17:09 ` Tadeusz Struk
0 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2021-11-18 16:38 UTC (permalink / raw)
To: Tadeusz Struk
Cc: David Miller, elver, nathan, ndesaulniers, jonathan.lemon,
alobakin, willemb, pabeni, cong.wang, haokexin, ilias.apalodimas,
netdev, linux-kernel, llvm, keescook, edumazet
On Thu, 18 Nov 2021 08:05:01 -0800 Tadeusz Struk wrote:
> On 11/12/21 07:42, David Miller wrote:
> > From: Jakub Kicinski <kuba@kernel.org>
> > Date: Thu, 11 Nov 2021 09:54:44 -0800
> >
> >> I'm not sure if that stalled due to lack of time or some fundamental
> >> problems.
> >
> > ran out of time, then had a stroke...
> >
> >> Seems like finishing that would let us clean up such misuses?
> >
> > yes it would
>
> so since there is not better way of suppressing the issue atm are
> you ok with taking this fix for now?
I vote no on sprinkling ugly tags around to silence some random
checkers warning. We already have too many of them. They are
meaningless and confusing to people reading the code.
This is not a fundamental problem, the solution is clear.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] skbuff: suppress clang object-size-mismatch error
2021-11-18 16:38 ` Jakub Kicinski
@ 2021-11-18 17:09 ` Tadeusz Struk
0 siblings, 0 replies; 10+ messages in thread
From: Tadeusz Struk @ 2021-11-18 17:09 UTC (permalink / raw)
To: Jakub Kicinski, David Miller
Cc: elver, nathan, ndesaulniers, jonathan.lemon, alobakin, willemb,
pabeni, cong.wang, haokexin, ilias.apalodimas, netdev,
linux-kernel, llvm, keescook, edumazet
On 11/18/21 08:38, Jakub Kicinski wrote:
> On Thu, 18 Nov 2021 08:05:01 -0800 Tadeusz Struk wrote:
>> On 11/12/21 07:42, David Miller wrote:
>>> From: Jakub Kicinski <kuba@kernel.org>
>>> Date: Thu, 11 Nov 2021 09:54:44 -0800
>>>
>>>> I'm not sure if that stalled due to lack of time or some fundamental
>>>> problems.
>>>
>>> ran out of time, then had a stroke...
>>>
>>>> Seems like finishing that would let us clean up such misuses?
>>>
>>> yes it would
>>
>> so since there is not better way of suppressing the issue atm are
>> you ok with taking this fix for now?
>
> I vote no on sprinkling ugly tags around to silence some random
> checkers warning. We already have too many of them. They are
> meaningless and confusing to people reading the code.
>
> This is not a fundamental problem, the solution is clear.
>
Fair enough.
David, did you post your work somewhere if someone would like to pick
it up and finish it?
--
Thanks,
Tadeusz
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-11-18 17:09 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-11 0:35 [PATCH] skbuff: suppress clang object-size-mismatch error Tadeusz Struk
2021-11-11 9:51 ` Marco Elver
2021-11-11 15:46 ` Tadeusz Struk
2021-11-11 15:52 ` Marco Elver
2021-11-11 16:01 ` Tadeusz Struk
2021-11-11 17:54 ` Jakub Kicinski
2021-11-12 15:42 ` David Miller
2021-11-18 16:05 ` Tadeusz Struk
2021-11-18 16:38 ` Jakub Kicinski
2021-11-18 17:09 ` Tadeusz Struk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.