| Message ID | 1492732760-25081-1-git-send-email-mhjungk@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Myungho Jung <mhjungk@gmail.com> Date: Thu, 20 Apr 2017 16:59:20 -0700 > Added NULL check to make __dev_kfree_skb_irq consistent with kfree > family of functions. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=195289 > > Signed-off-by: Myungho Jung <mhjungk@gmail.com> > --- > Changes in v2: > - Correct category in subject This subject line is an incomplete sentence. This patch prevents dereferenccing a null pointer when "what"?
On Mon, Apr 24, 2017 at 12:02:35PM -0400, David Miller wrote: > From: Myungho Jung <mhjungk@gmail.com> > Date: Thu, 20 Apr 2017 16:59:20 -0700 > > > Added NULL check to make __dev_kfree_skb_irq consistent with kfree > > family of functions. > > > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=195289 > > > > Signed-off-by: Myungho Jung <mhjungk@gmail.com> > > --- > > Changes in v2: > > - Correct category in subject > > This subject line is an incomplete sentence. > > This patch prevents dereferenccing a null pointer when "what"? As it was reported on bugzilla, it would happen when plugging p54 usb device to RPi2. But, i'm not 100% sure that this patch will resolve the issue because the reporter didn't try my patch yet and I don't have the device to test. And there might be some other places calling the function without checking null pointer. The thing is that only the function don't check null among kfree functions. So, I just hope this patch will prevent potential oops issues. Thanks, Myungho
On Mon, Apr 24, 2017 at 6:00 PM, Myungho Jung <mhjungk@gmail.com> wrote: > On Mon, Apr 24, 2017 at 12:02:35PM -0400, David Miller wrote: >> From: Myungho Jung <mhjungk@gmail.com> >> Date: Thu, 20 Apr 2017 16:59:20 -0700 >> >> > Added NULL check to make __dev_kfree_skb_irq consistent with kfree >> > family of functions. >> > >> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=195289 >> > >> > Signed-off-by: Myungho Jung <mhjungk@gmail.com> >> > --- >> > Changes in v2: >> > - Correct category in subject >> >> This subject line is an incomplete sentence. >> >> This patch prevents dereferenccing a null pointer when "what"? > > As it was reported on bugzilla, it would happen when plugging p54 usb device > to RPi2. But, i'm not 100% sure that this patch will resolve the issue because > the reporter didn't try my patch yet and I don't have the device to test. > > And there might be some other places calling the function without checking > null pointer. The thing is that only the function don't check null among > kfree functions. So, I just hope this patch will prevent potential oops > issues. Okay, but your patch title was : "net: core: Prevent from dereferencing null pointer when" "when" is usually followed by other words.
On Mon, Apr 24, 2017 at 06:10:32PM -0700, Eric Dumazet wrote: > On Mon, Apr 24, 2017 at 6:00 PM, Myungho Jung <mhjungk@gmail.com> wrote: > > On Mon, Apr 24, 2017 at 12:02:35PM -0400, David Miller wrote: > >> From: Myungho Jung <mhjungk@gmail.com> > >> Date: Thu, 20 Apr 2017 16:59:20 -0700 > >> > >> > Added NULL check to make __dev_kfree_skb_irq consistent with kfree > >> > family of functions. > >> > > >> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=195289 > >> > > >> > Signed-off-by: Myungho Jung <mhjungk@gmail.com> > >> > --- > >> > Changes in v2: > >> > - Correct category in subject > >> > >> This subject line is an incomplete sentence. > >> > >> This patch prevents dereferenccing a null pointer when "what"? > > > > As it was reported on bugzilla, it would happen when plugging p54 usb device > > to RPi2. But, i'm not 100% sure that this patch will resolve the issue because > > the reporter didn't try my patch yet and I don't have the device to test. > > > > And there might be some other places calling the function without checking > > null pointer. The thing is that only the function don't check null among > > kfree functions. So, I just hope this patch will prevent potential oops > > issues. > > Okay, but your patch title was : "net: core: Prevent from > dereferencing null pointer when" > > "when" is usually followed by other words. Oh, sorry that was typo. I'll remove "when" from subject. Thanks, Myungho
From: Myungho Jung <mhjungk@gmail.com> Date: Mon, 24 Apr 2017 18:00:52 -0700 > On Mon, Apr 24, 2017 at 12:02:35PM -0400, David Miller wrote: >> From: Myungho Jung <mhjungk@gmail.com> >> Date: Thu, 20 Apr 2017 16:59:20 -0700 >> >> > Added NULL check to make __dev_kfree_skb_irq consistent with kfree >> > family of functions. >> > >> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=195289 >> > >> > Signed-off-by: Myungho Jung <mhjungk@gmail.com> >> > --- >> > Changes in v2: >> > - Correct category in subject >> >> This subject line is an incomplete sentence. >> >> This patch prevents dereferenccing a null pointer when "what"? > > As it was reported on bugzilla, it would happen when plugging p54 usb device > to RPi2. But, i'm not 100% sure that this patch will resolve the issue because > the reporter didn't try my patch yet and I don't have the device to test. > > And there might be some other places calling the function without checking > null pointer. The thing is that only the function don't check null among > kfree functions. So, I just hope this patch will prevent potential oops > issues. It doesn't check for a NULL pointer because it is almost exclusively used in the interrupt paths where we know we have a non-NULL skb.
On Mon, Apr 24, 2017 at 09:44:50PM -0400, David Miller wrote: > From: Myungho Jung <mhjungk@gmail.com> > Date: Mon, 24 Apr 2017 18:00:52 -0700 > > > On Mon, Apr 24, 2017 at 12:02:35PM -0400, David Miller wrote: > >> From: Myungho Jung <mhjungk@gmail.com> > >> Date: Thu, 20 Apr 2017 16:59:20 -0700 > >> > >> > Added NULL check to make __dev_kfree_skb_irq consistent with kfree > >> > family of functions. > >> > > >> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=195289 > >> > > >> > Signed-off-by: Myungho Jung <mhjungk@gmail.com> > >> > --- > >> > Changes in v2: > >> > - Correct category in subject > >> > >> This subject line is an incomplete sentence. > >> > >> This patch prevents dereferenccing a null pointer when "what"? > > > > As it was reported on bugzilla, it would happen when plugging p54 usb device > > to RPi2. But, i'm not 100% sure that this patch will resolve the issue because > > the reporter didn't try my patch yet and I don't have the device to test. > > > > And there might be some other places calling the function without checking > > null pointer. The thing is that only the function don't check null among > > kfree functions. So, I just hope this patch will prevent potential oops > > issues. > > It doesn't check for a NULL pointer because it is almost exclusively > used in the interrupt paths where we know we have a non-NULL skb. Yes, actually null is checked before calling the function in most cases. That's why my first patch was applied not to net/core but to p54 driver because I was worried about duplicated checking. But, Christian suggested this patch to make it consistent with other kfree functions and consume_skb, and Eric agreed with that. Thanks, Myungho
diff --git a/net/core/dev.c b/net/core/dev.c index 7869ae3..22be2a6 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -2450,6 +2450,9 @@ void __dev_kfree_skb_irq(struct sk_buff *skb, enum skb_free_reason reason) { unsigned long flags; + if (unlikely(!skb)) + return; + if (likely(atomic_read(&skb->users) == 1)) { smp_rmb(); atomic_set(&skb->users, 0);
Added NULL check to make __dev_kfree_skb_irq consistent with kfree family of functions. Link: https://bugzilla.kernel.org/show_bug.cgi?id=195289 Signed-off-by: Myungho Jung <mhjungk@gmail.com> --- Changes in v2: - Correct category in subject net/core/dev.c | 3 +++ 1 file changed, 3 insertions(+)