* [PATCH V1 0/1] net: cdc_ncm: Fix TX zero padding
@ 2017-05-04 19:41 Jim Baxter
2017-05-04 19:41 ` [PATCH V1 1/1] " Jim Baxter
0 siblings, 1 reply; 6+ messages in thread
From: Jim Baxter @ 2017-05-04 19:41 UTC (permalink / raw)
To: linux-usb, linux-kernel, Oliver Neukum; +Cc: jim_baxter
Analysis
--------
The zero padding that is added to NTB's does not zero
the memory correctly.
This happens because the skb_put called within the memset in
the line:
memset(skb_put(skb_out, ctx->tx_max - skb_out->len),
0, ctx->tx_max - skb_out->len);
causes the value of skb_out->len to be modified during
the two uses of it within the above line.
This causes non-zeroed data at the end of skb_out.
This issue was found when connecting between an ARM
Sabre SD Host platform and a test box that was
dropping the NDP's due to the non zeroed memory being
identified as an error.
Solution
--------
To resolve this I have cached the value of
ctx->tx_max - skb_out->len before the memset operation.
Jim Baxter (1):
net: cdc_ncm: Fix TX zero padding
drivers/net/usb/cdc_ncm.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V1 1/1] net: cdc_ncm: Fix TX zero padding
2017-05-04 19:41 [PATCH V1 0/1] net: cdc_ncm: Fix TX zero padding Jim Baxter
@ 2017-05-04 19:41 ` Jim Baxter
2017-05-04 21:01 ` Bjørn Mork
0 siblings, 1 reply; 6+ messages in thread
From: Jim Baxter @ 2017-05-04 19:41 UTC (permalink / raw)
To: linux-usb, linux-kernel, Oliver Neukum; +Cc: jim_baxter
The zero padding that is added to NTB's does
not zero the memory correctly.
This is because the skb_put modifies the value
of skb_out->len which results in the memset
command not setting any memory to zero as
(ctx->tx_max - skb_out->len) == 0.
I have resolved this by storing the size of
the memory to be zeroed before the skb_put
and using this in the memset call.
Signed-off-by: Jim Baxter <jim_baxter@mentor.com>
---
drivers/net/usb/cdc_ncm.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index f317984..e2a48d7 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1087,6 +1087,7 @@ struct sk_buff *
u16 n = 0, index, ndplen;
u8 ready2send = 0;
u32 delayed_ndp_size;
+ size_t padding_count;
/* When our NDP gets written in cdc_ncm_ndp(), then skb_out->len gets updated
* accordingly. Otherwise, we should check here.
@@ -1243,11 +1244,13 @@ struct sk_buff *
* a ZLP after full sized NTBs.
*/
if (!(dev->driver_info->flags & FLAG_SEND_ZLP) &&
- skb_out->len > ctx->min_tx_pkt)
- memset(skb_put(skb_out, ctx->tx_max - skb_out->len), 0,
- ctx->tx_max - skb_out->len);
- else if (skb_out->len < ctx->tx_max && (skb_out->len % dev->maxpacket) == 0)
+ skb_out->len > ctx->min_tx_pkt) {
+ padding_count = ctx->tx_max - skb_out->len;
+ memset(skb_put(skb_out, padding_count), 0, padding_count);
+ } else if (skb_out->len < ctx->tx_max &&
+ (skb_out->len % dev->maxpacket) == 0) {
*skb_put(skb_out, 1) = 0; /* force short packet */
+ }
/* set final frame length */
nth16 = (struct usb_cdc_ncm_nth16 *)skb_out->data;
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V1 1/1] net: cdc_ncm: Fix TX zero padding
2017-05-04 19:41 ` [PATCH V1 1/1] " Jim Baxter
@ 2017-05-04 21:01 ` Bjørn Mork
2017-05-08 10:49 ` Baxter, Jim
0 siblings, 1 reply; 6+ messages in thread
From: Bjørn Mork @ 2017-05-04 21:01 UTC (permalink / raw)
To: Jim Baxter; +Cc: linux-usb, linux-kernel, Oliver Neukum
Jim Baxter <jim_baxter@mentor.com> writes:
> The zero padding that is added to NTB's does
> not zero the memory correctly.
> This is because the skb_put modifies the value
> of skb_out->len which results in the memset
> command not setting any memory to zero as
> (ctx->tx_max - skb_out->len) == 0.
>
> I have resolved this by storing the size of
> the memory to be zeroed before the skb_put
> and using this in the memset call.
>
> Signed-off-by: Jim Baxter <jim_baxter@mentor.com>
Ouch! Thanks for finding this. This should go to the stable queue as
well.
Reviewed-by: Bjørn Mork <bjorn@mork.no>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V1 1/1] net: cdc_ncm: Fix TX zero padding
2017-05-04 21:01 ` Bjørn Mork
@ 2017-05-08 10:49 ` Baxter, Jim
2017-05-08 11:08 ` Bjørn Mork
0 siblings, 1 reply; 6+ messages in thread
From: Baxter, Jim @ 2017-05-08 10:49 UTC (permalink / raw)
To: Bjørn Mork; +Cc: linux-usb, linux-kernel, Oliver Neukum
Bjørn Mork <bjorn@mork.no> writes:
>
> Ouch! Thanks for finding this. This should go to the stable queue as
> well.
>
> Reviewed-by: Bjørn Mork <bjorn@mork.no>
>
Do I need to submit this to the stable queue myself?
-- Jim
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V1 1/1] net: cdc_ncm: Fix TX zero padding
2017-05-08 10:49 ` Baxter, Jim
@ 2017-05-08 11:08 ` Bjørn Mork
2017-05-08 12:02 ` Baxter, Jim
0 siblings, 1 reply; 6+ messages in thread
From: Bjørn Mork @ 2017-05-08 11:08 UTC (permalink / raw)
To: Baxter, Jim; +Cc: linux-usb, linux-kernel, Oliver Neukum
"Baxter, Jim" <jim_baxter@mentor.com> writes:
> Bjørn Mork <bjorn@mork.no> writes:
>>
>> Ouch! Thanks for finding this. This should go to the stable queue as
>> well.
>>
>> Reviewed-by: Bjørn Mork <bjorn@mork.no>
>>
>
> Do I need to submit this to the stable queue myself?
No, davem will handle that.
That is, assuming that you had posted this to netdev in the first
place... Sorry, I just assumed you did without verifying it when I
replied. You will have to repost the patch to the appropriate
recepients. Use
scripts/get_maintainer.pl drivers/net/usb/cdc_ncm.c
for that. Feel free to include my Reviewed-by tag. And if you mark the
patch for "stable" somewhere outside the commit message, then davem will
pick that up and add it to his stable queue in patchwork. I don't know
if there is any officially documented way to do this, but a "stable" tag
inside the [] brackets in the subject, or a comment below the --- marker,
usually works fine.
Note that the "Cc: stable..." method isn't used for netdev patches. See
https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt
Bjørn
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V1 1/1] net: cdc_ncm: Fix TX zero padding
2017-05-08 11:08 ` Bjørn Mork
@ 2017-05-08 12:02 ` Baxter, Jim
0 siblings, 0 replies; 6+ messages in thread
From: Baxter, Jim @ 2017-05-08 12:02 UTC (permalink / raw)
To: Bjørn Mork; +Cc: linux-usb, linux-kernel, Oliver Neukum
Bjørn Mork <bjorn@mork.no> writes:
> "Baxter, Jim" <jim_baxter@mentor.com> writes:
>
>>
>> Do I need to submit this to the stable queue myself?
>
> No, davem will handle that.
>
> That is, assuming that you had posted this to netdev in the first
> place... Sorry, I just assumed you did without verifying it when I
> replied. You will have to repost the patch to the appropriate
> recepients. Use
>
> scripts/get_maintainer.pl drivers/net/usb/cdc_ncm.c
>
> for that. Feel free to include my Reviewed-by tag. And if you mark the
> patch for "stable" somewhere outside the commit message, then davem will
> pick that up and add it to his stable queue in patchwork. I don't know
> if there is any officially documented way to do this, but a "stable" tag
> inside the [] brackets in the subject, or a comment below the --- marker,
> usually works fine.
>
> Note that the "Cc: stable..." method isn't used for netdev patches. See
> https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt
>
>
> Bjørn
>
Thank you, I will resend this to netdev.
I had used the command:
scripts/get_maintainer.pl drivers/net/usb/cdc_ncm.c --sections
so only sent it to the lists in the first section "USB CDC ETHERNET DRIVER".
I will remove the --sections in future.
Jim
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-08 12:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-04 19:41 [PATCH V1 0/1] net: cdc_ncm: Fix TX zero padding Jim Baxter
2017-05-04 19:41 ` [PATCH V1 1/1] " Jim Baxter
2017-05-04 21:01 ` Bjørn Mork
2017-05-08 10:49 ` Baxter, Jim
2017-05-08 11:08 ` Bjørn Mork
2017-05-08 12:02 ` Baxter, Jim
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.