* [PATCH] net: sparx5: Fix return type of sparx5_port_xmit_impl
@ 2022-09-12 21:44 Nathan Huckleberry
2022-09-13 8:15 ` Casper Andersson
2022-09-20 14:30 ` Jakub Kicinski
0 siblings, 2 replies; 7+ messages in thread
From: Nathan Huckleberry @ 2022-09-12 21:44 UTC (permalink / raw)
Cc: Nathan Huckleberry, Dan Carpenter, llvm, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Lars Povlsen,
Steen Hegelund, UNGLinuxDriver, Nathan Chancellor,
Nick Desaulniers, Tom Rix, Horatiu Vultur, Casper Andersson,
netdev, linux-arm-kernel, linux-kernel
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 sparx5_port_xmit_impl should be changed from int to
netdev_tx_t.
Reported-by: Dan Carpenter <error27@gmail.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/1703
Cc: llvm@lists.linux.dev
Signed-off-by: Nathan Huckleberry <nhuck@google.com>
---
drivers/net/ethernet/microchip/sparx5/sparx5_packet.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
index 21844beba72d..83c16ca5b30f 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
@@ -222,13 +222,13 @@ static int sparx5_inject(struct sparx5 *sparx5,
return NETDEV_TX_OK;
}
-int sparx5_port_xmit_impl(struct sk_buff *skb, struct net_device *dev)
+netdev_tx_t sparx5_port_xmit_impl(struct sk_buff *skb, struct net_device *dev)
{
struct net_device_stats *stats = &dev->stats;
struct sparx5_port *port = netdev_priv(dev);
struct sparx5 *sparx5 = port->sparx5;
u32 ifh[IFH_LEN];
- int ret;
+ netdev_tx_t ret;
memset(ifh, 0, IFH_LEN * 4);
sparx5_set_port_ifh(ifh, port->portno);
--
2.37.2.789.g6183377224-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] net: sparx5: Fix return type of sparx5_port_xmit_impl
2022-09-12 21:44 [PATCH] net: sparx5: Fix return type of sparx5_port_xmit_impl Nathan Huckleberry
@ 2022-09-13 8:15 ` Casper Andersson
2022-09-13 20:46 ` Nathan Huckleberry
2022-09-20 14:29 ` Jakub Kicinski
2022-09-20 14:30 ` Jakub Kicinski
1 sibling, 2 replies; 7+ messages in thread
From: Casper Andersson @ 2022-09-13 8:15 UTC (permalink / raw)
To: Nathan Huckleberry
Cc: Dan Carpenter, llvm, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Nathan Chancellor, Nick Desaulniers, Tom Rix,
Horatiu Vultur, netdev, linux-arm-kernel, linux-kernel
Hi,
On 2022-09-12 14:44, 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 sparx5_port_xmit_impl should be changed from int to
> netdev_tx_t.
I noticed that the functions that assign the return value inside
sparx5_port_xmit_impl also have return type int, which would ideally
also be changed. But a bigger issue might be that
sparx5_ptp_txtstamp_request and sparx5_inject (called inside
sparx5_port_xmit_impl) returns -EBUSY (-16), when they should return
NETDEV_TX_BUSY (16). If this is an issue then it also needs to be fixed.
sparx5_fdma_xmit also has int return type, but always returns
NETDEV_TX_OK right now.
Best Regards,
Casper
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net: sparx5: Fix return type of sparx5_port_xmit_impl
2022-09-13 8:15 ` Casper Andersson
@ 2022-09-13 20:46 ` Nathan Huckleberry
2022-09-20 14:29 ` Jakub Kicinski
1 sibling, 0 replies; 7+ messages in thread
From: Nathan Huckleberry @ 2022-09-13 20:46 UTC (permalink / raw)
To: Casper Andersson
Cc: Dan Carpenter, llvm, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Nathan Chancellor, Nick Desaulniers, Tom Rix,
Horatiu Vultur, netdev, linux-arm-kernel, linux-kernel
Hey Casper,
On Tue, Sep 13, 2022 at 1:15 AM Casper Andersson <casper.casan@gmail.com> wrote:
>
> Hi,
>
> On 2022-09-12 14:44, 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 sparx5_port_xmit_impl should be changed from int to
> > netdev_tx_t.
>
> I noticed that the functions that assign the return value inside
> sparx5_port_xmit_impl also have return type int, which would ideally
> also be changed. But a bigger issue might be that
> sparx5_ptp_txtstamp_request and sparx5_inject (called inside
> sparx5_port_xmit_impl) returns -EBUSY (-16), when they should return
> NETDEV_TX_BUSY (16). If this is an issue then it also needs to be fixed.
It's not clear to me what happens when returning an error vs
NETDEV_TX_BUSY. The netdev_tx_t enum allows negative values, so
returning an error might be valid. If anyone more familiar with this
code has insight into why it's done like this, that'd be useful.
The important bit here for me is changing the return type on the
hooked function since the current code will cause panics.
>
> sparx5_fdma_xmit also has int return type, but always returns
> NETDEV_TX_OK right now.
>
> Best Regards,
> Casper
>
Thanks,
Huck
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net: sparx5: Fix return type of sparx5_port_xmit_impl
2022-09-13 8:15 ` Casper Andersson
2022-09-13 20:46 ` Nathan Huckleberry
@ 2022-09-20 14:29 ` Jakub Kicinski
1 sibling, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2022-09-20 14:29 UTC (permalink / raw)
To: Casper Andersson
Cc: Nathan Huckleberry, Dan Carpenter, llvm, David S. Miller,
Eric Dumazet, Paolo Abeni, Lars Povlsen, Steen Hegelund,
UNGLinuxDriver, Nathan Chancellor, Nick Desaulniers, Tom Rix,
Horatiu Vultur, netdev, linux-arm-kernel, linux-kernel
On Tue, 13 Sep 2022 10:15:48 +0200 Casper Andersson wrote:
> I noticed that the functions that assign the return value inside
> sparx5_port_xmit_impl also have return type int, which would ideally
> also be changed. But a bigger issue might be that
> sparx5_ptp_txtstamp_request and sparx5_inject (called inside
> sparx5_port_xmit_impl) returns -EBUSY (-16),
Yes, that seems off. IIUC error codes are treated as drops,
but the driver doesn't free the frame. So it's likely a leak.
> when they should return NETDEV_TX_BUSY (16). If this is an issue then
> it also needs to be fixed.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net: sparx5: Fix return type of sparx5_port_xmit_impl
2022-09-12 21:44 [PATCH] net: sparx5: Fix return type of sparx5_port_xmit_impl Nathan Huckleberry
2022-09-13 8:15 ` Casper Andersson
@ 2022-09-20 14:30 ` Jakub Kicinski
2022-09-29 18:19 ` [PATCH v2] " Nathan Huckleberry
1 sibling, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2022-09-20 14:30 UTC (permalink / raw)
To: Nathan Huckleberry
Cc: Dan Carpenter, llvm, David S. Miller, Eric Dumazet, Paolo Abeni,
Lars Povlsen, Steen Hegelund, UNGLinuxDriver, Nathan Chancellor,
Nick Desaulniers, Tom Rix, Horatiu Vultur, Casper Andersson,
netdev, linux-arm-kernel, linux-kernel
On Mon, 12 Sep 2022 14:44:29 -0700 Nathan Huckleberry wrote:
> -int sparx5_port_xmit_impl(struct sk_buff *skb, struct net_device *dev)
> +netdev_tx_t sparx5_port_xmit_impl(struct sk_buff *skb, struct net_device *dev)
Similarly to mana this has a prototype.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] net: sparx5: Fix return type of sparx5_port_xmit_impl
2022-09-20 14:30 ` Jakub Kicinski
@ 2022-09-29 18:19 ` Nathan Huckleberry
2022-10-03 9:50 ` patchwork-bot+netdevbpf
0 siblings, 1 reply; 7+ messages in thread
From: Nathan Huckleberry @ 2022-09-29 18:19 UTC (permalink / raw)
To: kuba
Cc: Steen.Hegelund, UNGLinuxDriver, casper.casan, davem, edumazet,
error27, horatiu.vultur, lars.povlsen, linux-arm-kernel,
linux-kernel, llvm, nathan, ndesaulniers, netdev, nhuck, pabeni,
trix
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 sparx5_port_xmit_impl should be changed from int to
netdev_tx_t.
Reported-by: Dan Carpenter <error27@gmail.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/1703
Cc: llvm@lists.linux.dev
Signed-off-by: Nathan Huckleberry <nhuck@google.com>
---
Changes v1 -> v2
- Updated header file
drivers/net/ethernet/microchip/sparx5/sparx5_main.h | 2 +-
drivers/net/ethernet/microchip/sparx5/sparx5_packet.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
index 8b42cad0e49c..7a83222caa73 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
@@ -305,7 +305,7 @@ struct frame_info {
void sparx5_xtr_flush(struct sparx5 *sparx5, u8 grp);
void sparx5_ifh_parse(u32 *ifh, struct frame_info *info);
irqreturn_t sparx5_xtr_handler(int irq, void *_priv);
-int sparx5_port_xmit_impl(struct sk_buff *skb, struct net_device *dev);
+netdev_tx_t sparx5_port_xmit_impl(struct sk_buff *skb, struct net_device *dev);
int sparx5_manual_injection_mode(struct sparx5 *sparx5);
void sparx5_port_inj_timer_setup(struct sparx5_port *port);
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
index 21844beba72d..83c16ca5b30f 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
@@ -222,13 +222,13 @@ static int sparx5_inject(struct sparx5 *sparx5,
return NETDEV_TX_OK;
}
-int sparx5_port_xmit_impl(struct sk_buff *skb, struct net_device *dev)
+netdev_tx_t sparx5_port_xmit_impl(struct sk_buff *skb, struct net_device *dev)
{
struct net_device_stats *stats = &dev->stats;
struct sparx5_port *port = netdev_priv(dev);
struct sparx5 *sparx5 = port->sparx5;
u32 ifh[IFH_LEN];
- int ret;
+ netdev_tx_t ret;
memset(ifh, 0, IFH_LEN * 4);
sparx5_set_port_ifh(ifh, port->portno);
--
2.38.0.rc1.362.ged0d419d3c-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] net: sparx5: Fix return type of sparx5_port_xmit_impl
2022-09-29 18:19 ` [PATCH v2] " Nathan Huckleberry
@ 2022-10-03 9:50 ` patchwork-bot+netdevbpf
0 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-10-03 9:50 UTC (permalink / raw)
To: Nathan Huckleberry
Cc: kuba, Steen.Hegelund, UNGLinuxDriver, casper.casan, davem,
edumazet, error27, horatiu.vultur, lars.povlsen,
linux-arm-kernel, linux-kernel, llvm, nathan, ndesaulniers,
netdev, pabeni, trix
Hello:
This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:
On Thu, 29 Sep 2022 11:19:47 -0700 you 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 sparx5_port_xmit_impl should be changed from int to
> netdev_tx_t.
>
> [...]
Here is the summary with links:
- [v2] net: sparx5: Fix return type of sparx5_port_xmit_impl
https://git.kernel.org/netdev/net/c/73ea73507359
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-10-03 9:51 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-12 21:44 [PATCH] net: sparx5: Fix return type of sparx5_port_xmit_impl Nathan Huckleberry
2022-09-13 8:15 ` Casper Andersson
2022-09-13 20:46 ` Nathan Huckleberry
2022-09-20 14:29 ` Jakub Kicinski
2022-09-20 14:30 ` Jakub Kicinski
2022-09-29 18:19 ` [PATCH v2] " Nathan Huckleberry
2022-10-03 9:50 ` patchwork-bot+netdevbpf
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).