* [PATCH] usbnet: Fix dma setup for fragmented packets that need a pad byte appended.
@ 2014-01-14 11:16 David Laight
2014-01-14 14:41 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: David Laight @ 2014-01-14 11:16 UTC (permalink / raw)
To: netdev
If the usbnet code appends a byte to a fragmented packet (in order to avoid
sending a bulk data message that is a multiple of the USB message size) then
the scatter-gather list isn't initialised correctly.
This causes a later panic in usb_hcd_map_urb_for_dma().
Basically when the code tries to access the final sg fragment the sg function
returns NULL because the 'end of sg list' market is set in the previous one.
Bug introduced in commit 60e453a940ac678565b6641d65f8c18541bb9f28
(USBNET: fix handling padding packet) and needs applying to all
kernels that contain this change (including 3.12).
Fix from Bjorn Mork.
Signed-off-by: David Laight <david.laight@aculab.com>
---
I think it is ok that the sg table's last element is never assigned to when
the packet isn't padded.
drivers/net/usb/usbnet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 8494bb5..aba04f5 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1245,7 +1245,7 @@ static int build_dma_sg(const struct sk_buff *skb, struct urb *urb)
return -ENOMEM;
urb->num_sgs = num_sgs;
- sg_init_table(urb->sg, urb->num_sgs);
+ sg_init_table(urb->sg, urb->num_sgs + 1);
sg_set_buf(&urb->sg[s++], skb->data, skb_headlen(skb));
total_len += skb_headlen(skb);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] usbnet: Fix dma setup for fragmented packets that need a pad byte appended.
2014-01-14 11:16 [PATCH] usbnet: Fix dma setup for fragmented packets that need a pad byte appended David Laight
@ 2014-01-14 14:41 ` Eric Dumazet
2014-01-14 14:48 ` David Laight
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2014-01-14 14:41 UTC (permalink / raw)
To: David Laight; +Cc: netdev
On Tue, 2014-01-14 at 11:16 +0000, David Laight wrote:
> If the usbnet code appends a byte to a fragmented packet (in order to avoid
> sending a bulk data message that is a multiple of the USB message size) then
> the scatter-gather list isn't initialised correctly.
> This causes a later panic in usb_hcd_map_urb_for_dma().
> Basically when the code tries to access the final sg fragment the sg function
> returns NULL because the 'end of sg list' market is set in the previous one.
>
> Bug introduced in commit 60e453a940ac678565b6641d65f8c18541bb9f28
> (USBNET: fix handling padding packet) and needs applying to all
> kernels that contain this change (including 3.12).
>
> Fix from Bjorn Mork.
>
> Signed-off-by: David Laight <david.laight@aculab.com>
> ---
>
> I think it is ok that the sg table's last element is never assigned to when
> the packet isn't padded.
Original patch contained :
Fixes: 60e453a940ac ("USBNET: fix handling padding packet")
Reported-by: Thomas Kear <thomas@kear.co.nz>
Cc: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
Why did you remove this and took ownership of this patch ?
It seems you would only need to either add :
Acked-by: ...
or
Tested-by: ...
or
Reviewed-by: ...
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] usbnet: Fix dma setup for fragmented packets that need a pad byte appended.
2014-01-14 14:41 ` Eric Dumazet
@ 2014-01-14 14:48 ` David Laight
2014-01-14 15:48 ` Bjørn Mork
0 siblings, 1 reply; 6+ messages in thread
From: David Laight @ 2014-01-14 14:48 UTC (permalink / raw)
To: 'Eric Dumazet'; +Cc: netdev
From: Eric Dumazet
> On Tue, 2014-01-14 at 11:16 +0000, David Laight wrote:
> > If the usbnet code appends a byte to a fragmented packet (in order to avoid
> > sending a bulk data message that is a multiple of the USB message size) then
> > the scatter-gather list isn't initialised correctly.
> > This causes a later panic in usb_hcd_map_urb_for_dma().
> > Basically when the code tries to access the final sg fragment the sg function
> > returns NULL because the 'end of sg list' market is set in the previous one.
> >
> > Bug introduced in commit 60e453a940ac678565b6641d65f8c18541bb9f28
> > (USBNET: fix handling padding packet) and needs applying to all
> > kernels that contain this change (including 3.12).
> >
> > Fix from Bjorn Mork.
> >
> > Signed-off-by: David Laight <david.laight@aculab.com>
> > ---
> >
> > I think it is ok that the sg table's last element is never assigned to when
> > the packet isn't padded.
>
> Original patch contained :
>
> Fixes: 60e453a940ac ("USBNET: fix handling padding packet")
> Reported-by: Thomas Kear <thomas@kear.co.nz>
> Cc: Ming Lei <ming.lei@canonical.com>
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
>
> Why did you remove this and took ownership of this patch ?
I couldn't find the original patch anywhere!
I did do quite a lot of looking as well - if I'd found it I've
have done something else.
In any case the intent was just to get the patch into 6.13 and stable
(for 6.12).
I don't care which version you apply.
Add a 'Reviewed by' for me if you want.
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] usbnet: Fix dma setup for fragmented packets that need a pad byte appended.
2014-01-14 14:48 ` David Laight
@ 2014-01-14 15:48 ` Bjørn Mork
2014-01-14 17:28 ` David Laight
0 siblings, 1 reply; 6+ messages in thread
From: Bjørn Mork @ 2014-01-14 15:48 UTC (permalink / raw)
To: David Laight; +Cc: 'Eric Dumazet', netdev
David Laight <David.Laight@ACULAB.COM> writes:
> I couldn't find the original patch anywhere!
> I did do quite a lot of looking as well - if I'd found it I've
> have done something else.
You obviously haven't looked the one place you are supposed to always
look before submitting *anything*:
https://git.kernel.org/cgit/linux/kernel/git/davem/net.git/commit/?id=fdc3452cd2c7b2bfe0f378f92123f4f9a98fa2bd
I am not impressed...
Bjørn
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] usbnet: Fix dma setup for fragmented packets that need a pad byte appended.
2014-01-14 15:48 ` Bjørn Mork
@ 2014-01-14 17:28 ` David Laight
2014-01-14 18:51 ` Bjørn Mork
0 siblings, 1 reply; 6+ messages in thread
From: David Laight @ 2014-01-14 17:28 UTC (permalink / raw)
To: 'Bjørn Mork'; +Cc: 'Eric Dumazet', netdev
From: Bjørn Mork [mailto:bjorn@mork.no]
> David Laight <David.Laight@ACULAB.COM> writes:
>
> > I couldn't find the original patch anywhere!
> > I did do quite a lot of looking as well - if I'd found it I've
> > have done something else.
>
> You obviously haven't looked the one place you are supposed to always
> look before submitting *anything*:
> https://git.kernel.org/cgit/linux/kernel/git/davem/net.git/commit/?id=fdc3452cd2c7b2bfe0f378f92123f4f9
> a98fa2bd
>
> I am not impressed...
Without the commit id that is rather hard.
Does anything even get there without manual actions.
usb patches sit in a hidden limbo for ages.
I looked through my mailboxes and searched for the thread - but only
found the messages that included the one that said you'd send a patch
soon.
What I still don't understand is how I found the patch before!
The fact that work make me use outluck makes searching mail locally
almost impossible.
I might have to subscribe at home as well.
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] usbnet: Fix dma setup for fragmented packets that need a pad byte appended.
2014-01-14 17:28 ` David Laight
@ 2014-01-14 18:51 ` Bjørn Mork
0 siblings, 0 replies; 6+ messages in thread
From: Bjørn Mork @ 2014-01-14 18:51 UTC (permalink / raw)
To: David Laight; +Cc: 'Eric Dumazet', netdev
David Laight <David.Laight@ACULAB.COM> writes:
> From: Bjørn Mork [mailto:bjorn@mork.no]
>> David Laight <David.Laight@ACULAB.COM> writes:
>>
>> > I couldn't find the original patch anywhere!
>> > I did do quite a lot of looking as well - if I'd found it I've
>> > have done something else.
>>
>> You obviously haven't looked the one place you are supposed to always
>> look before submitting *anything*:
>> https://git.kernel.org/cgit/linux/kernel/git/davem/net.git/commit/?id=fdc3452cd2c7b2bfe0f378f92123f4f9
>> a98fa2bd
>>
>> I am not impressed...
>
> Without the commit id that is rather hard.
I suggest you stop this nonsense now.
If you are going to submit a patch for "net", then the last thing you do
is check out net/master and rebase on top of it. If you had done so,
then you would have noticed the conflict. You never need to know the
commit id. Where the heck do you think I got it?
> Does anything even get there without manual actions.
> usb patches sit in a hidden limbo for ages.
>
> I looked through my mailboxes and searched for the thread - but only
> found the messages that included the one that said you'd send a patch
> soon.
>
> What I still don't understand is how I found the patch before!
Does it matter? You obviously knew you were posting a duplicate. There
is no excuse for that. Duplicates do *not* help. Whether you are able
to find the original or not doesn't matter. The rest of the world is.
Duplicate are confusing. Confusion cause delays and noise. I assume
you don't want either.
> The fact that work make me use outluck makes searching mail locally
> almost impossible.
Your lack of proper tools is your problem...
Now I know I come out a bit harsh. I can't help it. It's not that I
care much about whether or not I am credited for this oneliner, and you
did credit me in any case. But this whole discussion is so completely
unnecessary. Just use patchwork to track the status of patches you are
interested in. If you can't find a patch there, then YOU are doing
something wrong. It's as simple as that. No excuse.
Bjørn
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-01-14 18:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-14 11:16 [PATCH] usbnet: Fix dma setup for fragmented packets that need a pad byte appended David Laight
2014-01-14 14:41 ` Eric Dumazet
2014-01-14 14:48 ` David Laight
2014-01-14 15:48 ` Bjørn Mork
2014-01-14 17:28 ` David Laight
2014-01-14 18:51 ` Bjørn Mork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).