On Mon, Sep 12, 2022 at 02:45:56PM -0700, Nathan Huckleberry wrote:
> The ndo_start_xmit field in net_device_ops is expected to be of type
> netdev_tx_t (*ndo_start_xmit)(struct sk_buff *skb, struct net_device *dev).
>
> The mismatched return type breaks forward edge kCFI since the underlying
> function definition does not match the function hook definition.
>
> The return type of ieee80211_xmit should be changed from int to
> netdev_tx_t.
>
> Reported-by: Dan Carpenter <[email protected]>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1703
> Cc: [email protected]
> Signed-off-by: Nathan Huckleberry <[email protected]>
The protoype in drivers/staging/rtl8192u/ieee80211/ieee80211.h should be
updated as well. With that:
Reviewed-by: Nathan Chancellor <[email protected]>
> ---
> drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
> index 8602e3a6c837..e4b6454809a0 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
> @@ -526,7 +526,7 @@ static void ieee80211_query_seqnum(struct ieee80211_device *ieee,
> }
> }
>
> -int ieee80211_xmit(struct sk_buff *skb, struct net_device *dev)
> +netdev_tx_t ieee80211_xmit(struct sk_buff *skb, struct net_device *dev)
> {
> struct ieee80211_device *ieee = netdev_priv(dev);
> struct ieee80211_txb *txb = NULL;
> @@ -822,13 +822,13 @@ int ieee80211_xmit(struct sk_buff *skb, struct net_device *dev)
> if ((*ieee->hard_start_xmit)(txb, dev) == 0) {
> stats->tx_packets++;
> stats->tx_bytes += __le16_to_cpu(txb->payload_size);
> - return 0;
> + return NETDEV_TX_OK;
> }
> ieee80211_txb_free(txb);
> }
> }
>
> - return 0;
> + return NETDEV_TX_OK;
>
> failed:
> spin_unlock_irqrestore(&ieee->lock, flags);
I see 'return 1' down here, that doesn't appear to be a valid
'netdev_tx_t'. Should that be 'return NETDEV_TX_BUSY'? I guess that is
an outstanding issue though, just figured I would mention it.
> --
> 2.37.2.789.g6183377224-goog
>
Cheers,
Nathan
Hey Nathan,
On Tue, Sep 13, 2022 at 4:09 PM Nathan Chancellor <[email protected]> wrote:
>
> On Mon, Sep 12, 2022 at 02:45:56PM -0700, Nathan Huckleberry wrote:
> > The ndo_start_xmit field in net_device_ops is expected to be of type
> > netdev_tx_t (*ndo_start_xmit)(struct sk_buff *skb, struct net_device *dev).
> >
> > The mismatched return type breaks forward edge kCFI since the underlying
> > function definition does not match the function hook definition.
> >
> > The return type of ieee80211_xmit should be changed from int to
> > netdev_tx_t.
> >
> > Reported-by: Dan Carpenter <[email protected]>
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1703
> > Cc: [email protected]
> > Signed-off-by: Nathan Huckleberry <[email protected]>
>
> The protoype in drivers/staging/rtl8192u/ieee80211/ieee80211.h should be
> updated as well. With that:
Nice catch, thanks.
>
> Reviewed-by: Nathan Chancellor <[email protected]>
>
> > ---
> > drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
> > index 8602e3a6c837..e4b6454809a0 100644
> > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
> > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
> > @@ -526,7 +526,7 @@ static void ieee80211_query_seqnum(struct ieee80211_device *ieee,
> > }
> > }
> >
> > -int ieee80211_xmit(struct sk_buff *skb, struct net_device *dev)
> > +netdev_tx_t ieee80211_xmit(struct sk_buff *skb, struct net_device *dev)
> > {
> > struct ieee80211_device *ieee = netdev_priv(dev);
> > struct ieee80211_txb *txb = NULL;
> > @@ -822,13 +822,13 @@ int ieee80211_xmit(struct sk_buff *skb, struct net_device *dev)
> > if ((*ieee->hard_start_xmit)(txb, dev) == 0) {
> > stats->tx_packets++;
> > stats->tx_bytes += __le16_to_cpu(txb->payload_size);
> > - return 0;
> > + return NETDEV_TX_OK;
> > }
> > ieee80211_txb_free(txb);
> > }
> > }
> >
> > - return 0;
> > + return NETDEV_TX_OK;
> >
> > failed:
> > spin_unlock_irqrestore(&ieee->lock, flags);
>
> I see 'return 1' down here, that doesn't appear to be a valid
> 'netdev_tx_t'. Should that be 'return NETDEV_TX_BUSY'? I guess that is
> an outstanding issue though, just figured I would mention it.
I'm not sure. As far as I can tell, the dev_xmit_complete function in
include/linux/netdevice.h is used to determine whether an skb was
consumed or not. It looks like a value of 1 (NET_XMIT_DROP) would
signify that it was consumed, whereas NETDEV_TX_BUSY would not.
It's somewhat confusing that a function which returns an enum may also
return values outside of that enum.
For more info see the foillowing commits:
dc1f8bf68b311b1537cb65893430b6796118498a
572a9d7b6fc7f20f573664063324c086be310c42
7f2e870f2a48a0524a3b03b04fa019311d16a7f7
>
> > --
> > 2.37.2.789.g6183377224-goog
> >
>
> Cheers,
> Nathan
Thanks,
Huck
The ndo_start_xmit field in net_device_ops is expected to be of type
netdev_tx_t (*ndo_start_xmit)(struct sk_buff *skb, struct net_device *dev).
The mismatched return type breaks forward edge kCFI since the underlying
function definition does not match the function hook definition.
The return type of ieee80211_xmit should be changed from int to
netdev_tx_t.
Reported-by: Dan Carpenter <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
Link: https://github.com/ClangBuiltLinux/linux/issues/1703
Cc: [email protected]
Signed-off-by: Nathan Huckleberry <[email protected]>
---
Changes v1 -> v2:
- Updated header file
- Added reviewed-by tag
drivers/staging/rtl8192u/ieee80211/ieee80211.h | 2 +-
drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211.h b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
index b577f9c81f85..9cd4b1896745 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211.h
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
@@ -2178,7 +2178,7 @@ int ieee80211_set_encryption(struct ieee80211_device *ieee);
int ieee80211_encrypt_fragment(struct ieee80211_device *ieee,
struct sk_buff *frag, int hdr_len);
-int ieee80211_xmit(struct sk_buff *skb, struct net_device *dev);
+netdev_tx_t ieee80211_xmit(struct sk_buff *skb, struct net_device *dev);
void ieee80211_txb_free(struct ieee80211_txb *txb);
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
index 8602e3a6c837..e4b6454809a0 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c
@@ -526,7 +526,7 @@ static void ieee80211_query_seqnum(struct ieee80211_device *ieee,
}
}
-int ieee80211_xmit(struct sk_buff *skb, struct net_device *dev)
+netdev_tx_t ieee80211_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct ieee80211_device *ieee = netdev_priv(dev);
struct ieee80211_txb *txb = NULL;
@@ -822,13 +822,13 @@ int ieee80211_xmit(struct sk_buff *skb, struct net_device *dev)
if ((*ieee->hard_start_xmit)(txb, dev) == 0) {
stats->tx_packets++;
stats->tx_bytes += __le16_to_cpu(txb->payload_size);
- return 0;
+ return NETDEV_TX_OK;
}
ieee80211_txb_free(txb);
}
}
- return 0;
+ return NETDEV_TX_OK;
failed:
spin_unlock_irqrestore(&ieee->lock, flags);
--
2.37.2.789.g6183377224-goog