* [PATCH] staging:r8188eu: avoid skb_clone for amsdu to msdu conversion
@ 2020-04-18 8:41 Ivan Safonov
2020-04-23 11:29 ` Greg Kroah-Hartman
0 siblings, 1 reply; 3+ messages in thread
From: Ivan Safonov @ 2020-04-18 8:41 UTC (permalink / raw)
To: Larry Finger
Cc: Greg Kroah-Hartman, Puranjay Mohan, Saurav Girepunje, devel,
linux-kernel, Ivan Safonov
skb clones use same data buffer, so tail of one skb is corrupted by beginning of next skb.
Signed-off-by: Ivan Safonov <insafonov@gmail.com>
---
drivers/staging/rtl8188eu/core/rtw_recv.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c b/drivers/staging/rtl8188eu/core/rtw_recv.c
index d4278361e002..a036ef104198 100644
--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
@@ -1525,21 +1525,14 @@ static int amsdu_to_msdu(struct adapter *padapter, struct recv_frame *prframe)
/* Allocate new skb for releasing to upper layer */
sub_skb = dev_alloc_skb(nSubframe_Length + 12);
- if (sub_skb) {
- skb_reserve(sub_skb, 12);
- skb_put_data(sub_skb, pdata, nSubframe_Length);
- } else {
- sub_skb = skb_clone(prframe->pkt, GFP_ATOMIC);
- if (sub_skb) {
- sub_skb->data = pdata;
- sub_skb->len = nSubframe_Length;
- skb_set_tail_pointer(sub_skb, nSubframe_Length);
- } else {
- DBG_88E("skb_clone() Fail!!! , nr_subframes=%d\n", nr_subframes);
- break;
- }
+ if (!sub_skb) {
+ DBG_88E("dev_alloc_skb() Fail!!! , nr_subframes=%d\n", nr_subframes);
+ break;
}
+ skb_reserve(sub_skb, 12);
+ skb_put_data(sub_skb, pdata, nSubframe_Length);
+
subframes[nr_subframes++] = sub_skb;
if (nr_subframes >= MAX_SUBFRAME_COUNT) {
--
2.24.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] staging:r8188eu: avoid skb_clone for amsdu to msdu conversion
2020-04-18 8:41 [PATCH] staging:r8188eu: avoid skb_clone for amsdu to msdu conversion Ivan Safonov
@ 2020-04-23 11:29 ` Greg Kroah-Hartman
2020-04-23 13:02 ` Ivan Safonov
0 siblings, 1 reply; 3+ messages in thread
From: Greg Kroah-Hartman @ 2020-04-23 11:29 UTC (permalink / raw)
To: Ivan Safonov
Cc: Larry Finger, devel, Puranjay Mohan, linux-kernel, Saurav Girepunje
On Sat, Apr 18, 2020 at 11:41:12AM +0300, Ivan Safonov wrote:
> skb clones use same data buffer, so tail of one skb is corrupted by beginning of next skb.
Please properly wrap your changelog text at the correct column (72).
Also, your subject: line does not have the correct driver name :(
>
> Signed-off-by: Ivan Safonov <insafonov@gmail.com>
> ---
> drivers/staging/rtl8188eu/core/rtw_recv.c | 19 ++++++-------------
> 1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c b/drivers/staging/rtl8188eu/core/rtw_recv.c
> index d4278361e002..a036ef104198 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_recv.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
> @@ -1525,21 +1525,14 @@ static int amsdu_to_msdu(struct adapter *padapter, struct recv_frame *prframe)
>
> /* Allocate new skb for releasing to upper layer */
> sub_skb = dev_alloc_skb(nSubframe_Length + 12);
> - if (sub_skb) {
> - skb_reserve(sub_skb, 12);
> - skb_put_data(sub_skb, pdata, nSubframe_Length);
> - } else {
> - sub_skb = skb_clone(prframe->pkt, GFP_ATOMIC);
> - if (sub_skb) {
> - sub_skb->data = pdata;
> - sub_skb->len = nSubframe_Length;
> - skb_set_tail_pointer(sub_skb, nSubframe_Length);
> - } else {
> - DBG_88E("skb_clone() Fail!!! , nr_subframes=%d\n", nr_subframes);
> - break;
> - }
> + if (!sub_skb) {
> + DBG_88E("dev_alloc_skb() Fail!!! , nr_subframes=%d\n", nr_subframes);
> + break;
> }
>
> + skb_reserve(sub_skb, 12);
> + skb_put_data(sub_skb, pdata, nSubframe_Length);
> +
Have you tested this?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] staging:r8188eu: avoid skb_clone for amsdu to msdu conversion
2020-04-23 11:29 ` Greg Kroah-Hartman
@ 2020-04-23 13:02 ` Ivan Safonov
0 siblings, 0 replies; 3+ messages in thread
From: Ivan Safonov @ 2020-04-23 13:02 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Larry Finger, devel, Puranjay Mohan, linux-kernel, Saurav Girepunje
On 4/23/20 2:29 PM, Greg Kroah-Hartman wrote:
> On Sat, Apr 18, 2020 at 11:41:12AM +0300, Ivan Safonov wrote:
>> skb clones use same data buffer, so tail of one skb is corrupted by beginning of next skb.
>
> Please properly wrap your changelog text at the correct column (72).
>
> Also, your subject: line does not have the correct driver name :(
Correct driver name is 'r8188eu':
1.
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2016-June/090556.html
:
> One other point, it is customary to start the patch subject with "staging:
> rtl8188eu: ..." for drivers in staging. I also prefer using r8188eu rather than
> rtl8188eu as the former is the actual name of the driver, but either will work.
2. ./drivers/staging/rtl8188eu/os_dep/usb_intf.c lines 483-522/522 (END):
> static struct usb_driver rtl8188e_usb_drv = {
> .name = "r8188eu",
> .probe = rtw_drv_init,
> .disconnect = rtw_dev_remove,
> .id_table = rtw_usb_id_tbl,
> .suspend = rtw_suspend,
> .resume = rtw_resume,
> .reset_resume = rtw_resume,
> };
>
> module_usb_driver(rtl8188e_usb_drv)
Subject contain this name.
>>
>> Signed-off-by: Ivan Safonov <insafonov@gmail.com>
>> ---
>> drivers/staging/rtl8188eu/core/rtw_recv.c | 19 ++++++-------------
>> 1 file changed, 6 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c b/drivers/staging/rtl8188eu/core/rtw_recv.c
>> index d4278361e002..a036ef104198 100644
>> --- a/drivers/staging/rtl8188eu/core/rtw_recv.c
>> +++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
>> @@ -1525,21 +1525,14 @@ static int amsdu_to_msdu(struct adapter *padapter, struct recv_frame *prframe)
>>
>> /* Allocate new skb for releasing to upper layer */
>> sub_skb = dev_alloc_skb(nSubframe_Length + 12);
>> - if (sub_skb) {
>> - skb_reserve(sub_skb, 12);
>> - skb_put_data(sub_skb, pdata, nSubframe_Length);
>> - } else {
>> - sub_skb = skb_clone(prframe->pkt, GFP_ATOMIC);
>> - if (sub_skb) {
>> - sub_skb->data = pdata;
>> - sub_skb->len = nSubframe_Length;
>> - skb_set_tail_pointer(sub_skb, nSubframe_Length);
>> - } else {
>> - DBG_88E("skb_clone() Fail!!! , nr_subframes=%d\n", nr_subframes);
>> - break;
>> - }
>> + if (!sub_skb) {
>> + DBG_88E("dev_alloc_skb() Fail!!! , nr_subframes=%d\n", nr_subframes);
>> + break;
>> }
>>
>> + skb_reserve(sub_skb, 12);
>> + skb_put_data(sub_skb, pdata, nSubframe_Length);
>> +
>
> Have you tested this?
I have not test this change.
Ivan Safonov.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-04-23 12:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-18 8:41 [PATCH] staging:r8188eu: avoid skb_clone for amsdu to msdu conversion Ivan Safonov
2020-04-23 11:29 ` Greg Kroah-Hartman
2020-04-23 13:02 ` Ivan Safonov
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).