[RESEND PATCH] staging: r8188eu: Fix return type for implementation of ndo_start_xmit

[RESEND PATCH] staging: r8188eu: Fix return type for implementation of ndo_start_xmit

[GUO Zihua]

Since Linux now supports CFI, it will be a good idea to fix mismatched
return type for implementation of hooks. Otherwise this might get
cought out by CFI and cause a panic.

Use enums from netdev_tx_t as return value instead, then change return
type to netdev_tx_t.

Signed-off-by: GUO Zihua <guozihua@huawei.com>
---
 drivers/staging/r8188eu/include/xmit_osdep.h | 2 +-
 drivers/staging/r8188eu/os_dep/xmit_linux.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/r8188eu/include/xmit_osdep.h b/drivers/staging/r8188eu/include/xmit_osdep.h
index 00658681fef9..947242486144 100644
--- a/drivers/staging/r8188eu/include/xmit_osdep.h
+++ b/drivers/staging/r8188eu/include/xmit_osdep.h
@@ -28,7 +28,7 @@ struct sta_xmit_priv;
 struct xmit_frame;
 struct xmit_buf;
 
-int rtw_xmit_entry(struct sk_buff *pkt, struct  net_device *pnetdev);
+netdev_tx_t rtw_xmit_entry(struct sk_buff *pkt, struct  net_device *pnetdev);
 
 void rtw_os_xmit_schedule(struct adapter *padapter);
 
diff --git a/drivers/staging/r8188eu/os_dep/xmit_linux.c b/drivers/staging/r8188eu/os_dep/xmit_linux.c
index 91a1e4e3219a..0b04010d6d82 100644
--- a/drivers/staging/r8188eu/os_dep/xmit_linux.c
+++ b/drivers/staging/r8188eu/os_dep/xmit_linux.c
@@ -198,7 +198,7 @@ static int rtw_mlcst2unicst(struct adapter *padapter, struct sk_buff *skb)
 	return true;
 }
 
-int rtw_xmit_entry(struct sk_buff *pkt, struct  net_device *pnetdev)
+netdev_tx_t rtw_xmit_entry(struct sk_buff *pkt, struct  net_device *pnetdev)
 {
 	struct adapter *padapter = (struct adapter *)rtw_netdev_priv(pnetdev);
 	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
@@ -233,5 +233,5 @@ int rtw_xmit_entry(struct sk_buff *pkt, struct  net_device *pnetdev)
 
 exit:
 
-	return 0;
+	return NETDEV_TX_OK;
 }
-- 
2.17.1

Re: [RESEND PATCH] staging: r8188eu: Fix return type for implementation of ndo_start_xmit

[Greg KH]

On Fri, Sep 02, 2022 at 04:16:53PM +0800, GUO Zihua wrote:
> Since Linux now supports CFI, it will be a good idea to fix mismatched
> return type for implementation of hooks. Otherwise this might get
> cought out by CFI and cause a panic.
> 
> Use enums from netdev_tx_t as return value instead, then change return
> type to netdev_tx_t.
> 
> Signed-off-by: GUO Zihua <guozihua@huawei.com>
> ---
>  drivers/staging/r8188eu/include/xmit_osdep.h | 2 +-
>  drivers/staging/r8188eu/os_dep/xmit_linux.c  | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

Why is this and the other commits, marked as RESEND?

And any reason you didn't cc: the staging maintainer on this commit?

confused,

greg k-h

Re: [RESEND PATCH] staging: r8188eu: Fix return type for implementation of ndo_start_xmit

[Dan Carpenter]

This is not a RESEND.  A RESEND means your patch was perfect and the
problem was on our end.  Of course, we're also human so sometimes we
make mistakes but a RESEND requires an investigation why we are
ignoring *perfect* patches.

This is a V2 patch.  Here is my blog entry on how to send a v2 patch:
https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/
You need to send a v3 patch now.

On Fri, Sep 02, 2022 at 04:16:53PM +0800, GUO Zihua wrote:
> Since Linux now supports CFI, it will be a good idea to fix mismatched
> return type for implementation of hooks. Otherwise this might get
> cought out by CFI and cause a panic.
> 
> Use enums from netdev_tx_t as return value instead, then change return
> type to netdev_tx_t.

What CFI is C-something Fault Injection?  How does this cause a panic?
Do you have a stack trace?  This commit message is not clear at all.

regards,
dan carpenter

Re: [RESEND PATCH] staging: r8188eu: Fix return type for implementation of ndo_start_xmit

[Guozihua (Scott)]

Hi Greg,

Sorry for the confusion.

On 2022/9/2 16:25, Greg KH wrote:
> On Fri, Sep 02, 2022 at 04:16:53PM +0800, GUO Zihua wrote:
>> Since Linux now supports CFI, it will be a good idea to fix mismatched
>> return type for implementation of hooks. Otherwise this might get
>> cought out by CFI and cause a panic.
>>
>> Use enums from netdev_tx_t as return value instead, then change return
>> type to netdev_tx_t.
>>
>> Signed-off-by: GUO Zihua <guozihua@huawei.com>
>> ---
>>   drivers/staging/r8188eu/include/xmit_osdep.h | 2 +-
>>   drivers/staging/r8188eu/os_dep/xmit_linux.c  | 4 ++--
>>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> Why is this and the other commits, marked as RESEND?

The first version of the patch has a truncated subject, thus the resend. 
It might be better to send a v2 patches instead..
> 
> And any reason you didn't cc: the staging maintainer on this commit?

It's my first time sending patches to the staging maillist. When I run 
get_maintainer.pl on the patch I don't see a "maintainer", but only 
"supporters" and "reviewers". Is that for staging there are a group of 
maintainers for all modules?
> 
> confused,
> 
> greg k-h
> .
-- 
Best
GUO Zihua

Re: [RESEND PATCH] staging: r8188eu: Fix return type for implementation of ndo_start_xmit

[Greg KH]

On Mon, Sep 05, 2022 at 09:23:15AM +0800, Guozihua (Scott) wrote:
> Hi Greg,
> 
> Sorry for the confusion.
> 
> On 2022/9/2 16:25, Greg KH wrote:
> > On Fri, Sep 02, 2022 at 04:16:53PM +0800, GUO Zihua wrote:
> > > Since Linux now supports CFI, it will be a good idea to fix mismatched
> > > return type for implementation of hooks. Otherwise this might get
> > > cought out by CFI and cause a panic.
> > > 
> > > Use enums from netdev_tx_t as return value instead, then change return
> > > type to netdev_tx_t.
> > > 
> > > Signed-off-by: GUO Zihua <guozihua@huawei.com>
> > > ---
> > >   drivers/staging/r8188eu/include/xmit_osdep.h | 2 +-
> > >   drivers/staging/r8188eu/os_dep/xmit_linux.c  | 4 ++--
> > >   2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > Why is this and the other commits, marked as RESEND?
> 
> The first version of the patch has a truncated subject, thus the resend. It
> might be better to send a v2 patches instead..

Yes please do that.

> > And any reason you didn't cc: the staging maintainer on this commit?
> 
> It's my first time sending patches to the staging maillist. When I run
> get_maintainer.pl on the patch I don't see a "maintainer", but only
> "supporters" and "reviewers". Is that for staging there are a group of
> maintainers for all modules?

See the MAINTAINERS file for what "supported" and "maintained" and the
rest mean and just include them all when you submit a patch.

thanks,

greg k-h

Re: [RESEND PATCH] staging: r8188eu: Fix return type for implementation of ndo_start_xmit

[Guozihua (Scott)]

Hi Greg,

On 2022/9/5 13:16, Greg KH wrote:
> On Mon, Sep 05, 2022 at 09:23:15AM +0800, Guozihua (Scott) wrote:
>> Hi Greg,
>>
>> Sorry for the confusion.
>>
>> On 2022/9/2 16:25, Greg KH wrote:
>>> On Fri, Sep 02, 2022 at 04:16:53PM +0800, GUO Zihua wrote:
>>>> Since Linux now supports CFI, it will be a good idea to fix mismatched
>>>> return type for implementation of hooks. Otherwise this might get
>>>> cought out by CFI and cause a panic.
>>>>
>>>> Use enums from netdev_tx_t as return value instead, then change return
>>>> type to netdev_tx_t.
>>>>
>>>> Signed-off-by: GUO Zihua <guozihua@huawei.com>
>>>> ---
>>>>    drivers/staging/r8188eu/include/xmit_osdep.h | 2 +-
>>>>    drivers/staging/r8188eu/os_dep/xmit_linux.c  | 4 ++--
>>>>    2 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> Why is this and the other commits, marked as RESEND?
>>
>> The first version of the patch has a truncated subject, thus the resend. It
>> might be better to send a v2 patches instead..
> 
> Yes please do that.

The patch is sent as V3, as the RESEND patches should be considered as 
V2 as Dan suggested.
> 
>>> And any reason you didn't cc: the staging maintainer on this commit?
>>
>> It's my first time sending patches to the staging maillist. When I run
>> get_maintainer.pl on the patch I don't see a "maintainer", but only
>> "supporters" and "reviewers". Is that for staging there are a group of
>> maintainers for all modules?
> 
> See the MAINTAINERS file for what "supported" and "maintained" and the
> rest mean and just include them all when you submit a patch.
> 
> thanks,
> 
> greg k-h
> .

I check the MAINTAINER file and I would include everyone with supported 
and maintained role.

-- 
Best
GUO Zihua