* [PATCH 0/6] can: add support for ETAS ES58X CAN USB
@ 2020-09-26 17:29 Vincent Mailhol
2020-09-26 17:41 ` [PATCH 1/6] can: dev: can_get_echo_skb(): prevent call to kfree_skb() in hard IRQ context Vincent Mailhol
0 siblings, 1 reply; 4+ messages in thread
From: Vincent Mailhol @ 2020-09-26 17:29 UTC (permalink / raw)
To: linux-can, Wolfgang Grandegger, Marc Kleine-Budde, David S . Miller
Cc: Vincent Mailhol
The purpose of this patch series is to introduce a new CAN USB
driver to support ETAS USB interfaces (ES58X series).
During development, issues in drivers/net/can/dev.c where discovered,
the fix for those issues are included in this patch series.
We also propose to add two helper functions in include/linux/can/dev.h
which we think can benefit other drivers: get_can_len() and
can_bit_time().
The driver indirectly relies on https://lkml.org/lkml/2020/9/26/251
([PATCH] can: raw: add missing error queue support) for the call to
skb_tx_timestamp() to work but can still compile without it.
*Side notes*: scripts/checkpatch.pl returns 4 'checks' findings in
[PATCH 5/6]. All those findings are of type: "Macro argument reuse 'x'
possible side-effects?". Those arguments reuse are actually made by
calling either __stringify() or sizeof_field() which are both
pre-processor constant. Furthermore, those macro are never called with
arguments sensible to side-effects. So no actual side effect would
occur.
Thank you for your comments.
Vincent Mailhol (6):
can: dev: can_get_echo_skb(): prevent call to kfree_skb() in hard IRQ
context
can: dev: add a helper function to get the correct length of Classical
frames
can: dev: __can_get_echo_skb(): fix the return length
can: dev: add a helper function to calculate the duration of one bit
can: usb: etas_es58X: add support for ETAS ES58X CAN USB interfaces
USB: cdc-acm: blacklist ETAS ES58X device
drivers/net/can/dev.c | 26 +-
drivers/net/can/usb/Kconfig | 9 +
drivers/net/can/usb/Makefile | 1 +
drivers/net/can/usb/etas_es58x/Makefile | 4 +
drivers/net/can/usb/etas_es58x/es581_4.c | 560 ++++
drivers/net/can/usb/etas_es58x/es581_4.h | 237 ++
drivers/net/can/usb/etas_es58x/es58x_core.c | 2725 +++++++++++++++++++
drivers/net/can/usb/etas_es58x/es58x_core.h | 700 +++++
drivers/net/can/usb/etas_es58x/es58x_fd.c | 650 +++++
drivers/net/can/usb/etas_es58x/es58x_fd.h | 243 ++
drivers/usb/class/cdc-acm.c | 11 +
include/linux/can/dev.h | 38 +
12 files changed, 5190 insertions(+), 14 deletions(-)
create mode 100644 drivers/net/can/usb/etas_es58x/Makefile
create mode 100644 drivers/net/can/usb/etas_es58x/es581_4.c
create mode 100644 drivers/net/can/usb/etas_es58x/es581_4.h
create mode 100644 drivers/net/can/usb/etas_es58x/es58x_core.c
create mode 100644 drivers/net/can/usb/etas_es58x/es58x_core.h
create mode 100644 drivers/net/can/usb/etas_es58x/es58x_fd.c
create mode 100644 drivers/net/can/usb/etas_es58x/es58x_fd.h
--
2.26.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/6] can: dev: can_get_echo_skb(): prevent call to kfree_skb() in hard IRQ context
2020-09-26 17:29 [PATCH 0/6] can: add support for ETAS ES58X CAN USB Vincent Mailhol
@ 2020-09-26 17:41 ` Vincent Mailhol
0 siblings, 0 replies; 4+ messages in thread
From: Vincent Mailhol @ 2020-09-26 17:41 UTC (permalink / raw)
To: linux-can, Wolfgang Grandegger, Marc Kleine-Budde, David S . Miller
Cc: Vincent Mailhol, Jakub Kicinski, netdev, linux-kernel
If a driver calls can_get_echo_skb() during a hardware IRQ (which is
often, but not always, the case), the 'WARN_ON(in_irq)' in
net/core/skbuff.c#skb_release_head_state() might be triggered, under
network congestion circumstances, together with the potential risk of
a NULL pointer dereference.
The root cause of this issue is the call to kfree_skb() instead of
dev_kfree_skb_irq() in net/core/dev.c#enqueue_to_backlog().
This patch prevents the skb to be freed within the call to netif_rx()
by incrementing its reference count with skb_get(). The skb is finally
freed by one of the in-irq-context safe functions:
dev_consume_skb_any() or dev_kfree_skb_any(). The "any" version is
used because some drivers might call can_get_echo_skb() in a normal
context.
The reason for this issue to occur is that initially, in the core
network stack, loopback skb were not supposed to be received in
hardware IRQ context. The CAN stack is an exeption.
This bug is exactly what is described in
https://patchwork.ozlabs.org/patch/835236/
While above link proposes a patch that directly modifies
net/core/dev.c, we try to propose here a smoother modification local
to CAN network stack (the assumption behind is that only CAN devices
are affected by this issue).
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
drivers/net/can/dev.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 68834a2853c9..e291fda395a0 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -512,7 +512,11 @@ unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx)
if (!skb)
return 0;
- netif_rx(skb);
+ skb_get(skb);
+ if (netif_rx(skb) == NET_RX_SUCCESS)
+ dev_consume_skb_any(skb);
+ else
+ dev_kfree_skb_any(skb);
return len;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 0/6] can: add support for ETAS ES58X CAN USB
@ 2020-09-26 17:57 Vincent Mailhol
0 siblings, 0 replies; 4+ messages in thread
From: Vincent Mailhol @ 2020-09-26 17:57 UTC (permalink / raw)
To: linux-kernel, netdev, linux-can, Wolfgang Grandegger,
Marc Kleine-Budde, David S . Miller
Cc: Vincent Mailhol
Resending: In my previous e-mail, I forgot to include the
linux-kernel@vger.kernel.org in the cover letter which broke the chain
reply... Sorry for the spam.
The purpose of this patch series is to introduce a new CAN USB
driver to support ETAS USB interfaces (ES58X series).
During development, issues in drivers/net/can/dev.c where discovered,
the fix for those issues are included in this patch series.
We also propose to add two helper functions in include/linux/can/dev.h
which we think can benefit other drivers: get_can_len() and
can_bit_time().
The driver indirectly relies on https://lkml.org/lkml/2020/9/26/251
([PATCH] can: raw: add missing error queue support) for the call to
skb_tx_timestamp() to work but can still compile without it.
*Side notes*: scripts/checkpatch.pl returns 4 'checks' findings in
[PATCH 5/6]. All those findings are of type: "Macro argument reuse 'x'
possible side-effects?". Those arguments reuse are actually made by
calling either __stringify() or sizeof_field() which are both
pre-processor constant. Furthermore, those macro are never called with
arguments sensible to side-effects. So no actual side effect would
occur.
Thank you for your comments.
Vincent Mailhol (6):
can: dev: can_get_echo_skb(): prevent call to kfree_skb() in hard IRQ
context
can: dev: add a helper function to get the correct length of Classical
frames
can: dev: __can_get_echo_skb(): fix the return length
can: dev: add a helper function to calculate the duration of one bit
can: usb: etas_es58X: add support for ETAS ES58X CAN USB interfaces
USB: cdc-acm: blacklist ETAS ES58X device
drivers/net/can/dev.c | 26 +-
drivers/net/can/usb/Kconfig | 9 +
drivers/net/can/usb/Makefile | 1 +
drivers/net/can/usb/etas_es58x/Makefile | 4 +
drivers/net/can/usb/etas_es58x/es581_4.c | 560 ++++
drivers/net/can/usb/etas_es58x/es581_4.h | 237 ++
drivers/net/can/usb/etas_es58x/es58x_core.c | 2725 +++++++++++++++++++
drivers/net/can/usb/etas_es58x/es58x_core.h | 700 +++++
drivers/net/can/usb/etas_es58x/es58x_fd.c | 650 +++++
drivers/net/can/usb/etas_es58x/es58x_fd.h | 243 ++
drivers/usb/class/cdc-acm.c | 11 +
include/linux/can/dev.h | 38 +
12 files changed, 5190 insertions(+), 14 deletions(-)
create mode 100644 drivers/net/can/usb/etas_es58x/Makefile
create mode 100644 drivers/net/can/usb/etas_es58x/es581_4.c
create mode 100644 drivers/net/can/usb/etas_es58x/es581_4.h
create mode 100644 drivers/net/can/usb/etas_es58x/es58x_core.c
create mode 100644 drivers/net/can/usb/etas_es58x/es58x_core.h
create mode 100644 drivers/net/can/usb/etas_es58x/es58x_fd.c
create mode 100644 drivers/net/can/usb/etas_es58x/es58x_fd.h
--
2.26.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 0/6] can: add support for ETAS ES58X CAN USB
@ 2020-09-26 17:45 Vincent Mailhol
0 siblings, 0 replies; 4+ messages in thread
From: Vincent Mailhol @ 2020-09-26 17:45 UTC (permalink / raw)
To: linux-can, Wolfgang Grandegger, Marc Kleine-Budde, David S . Miller
Cc: Vincent Mailhol
The purpose of this patch series is to introduce a new CAN USB
driver to support ETAS USB interfaces (ES58X series).
During development, issues in drivers/net/can/dev.c where discovered,
the fix for those issues are included in this patch series.
We also propose to add two helper functions in include/linux/can/dev.h
which we think can benefit other drivers: get_can_len() and
can_bit_time().
The driver indirectly relies on https://lkml.org/lkml/2020/9/26/251
([PATCH] can: raw: add missing error queue support) for the call to
skb_tx_timestamp() to work but can still compile without it.
*Side notes*: scripts/checkpatch.pl returns 4 'checks' findings in
[PATCH 5/6]. All those findings are of type: "Macro argument reuse 'x'
possible side-effects?". Those arguments reuse are actually made by
calling either __stringify() or sizeof_field() which are both
pre-processor constant. Furthermore, those macro are never called with
arguments sensible to side-effects. So no actual side effect would
occur.
Thank you for your comments.
Vincent Mailhol (6):
can: dev: can_get_echo_skb(): prevent call to kfree_skb() in hard IRQ
context
can: dev: add a helper function to get the correct length of Classical
frames
can: dev: __can_get_echo_skb(): fix the return length
can: dev: add a helper function to calculate the duration of one bit
can: usb: etas_es58X: add support for ETAS ES58X CAN USB interfaces
USB: cdc-acm: blacklist ETAS ES58X device
drivers/net/can/dev.c | 26 +-
drivers/net/can/usb/Kconfig | 9 +
drivers/net/can/usb/Makefile | 1 +
drivers/net/can/usb/etas_es58x/Makefile | 4 +
drivers/net/can/usb/etas_es58x/es581_4.c | 560 ++++
drivers/net/can/usb/etas_es58x/es581_4.h | 237 ++
drivers/net/can/usb/etas_es58x/es58x_core.c | 2725 +++++++++++++++++++
drivers/net/can/usb/etas_es58x/es58x_core.h | 700 +++++
drivers/net/can/usb/etas_es58x/es58x_fd.c | 650 +++++
drivers/net/can/usb/etas_es58x/es58x_fd.h | 243 ++
drivers/usb/class/cdc-acm.c | 11 +
include/linux/can/dev.h | 38 +
12 files changed, 5190 insertions(+), 14 deletions(-)
create mode 100644 drivers/net/can/usb/etas_es58x/Makefile
create mode 100644 drivers/net/can/usb/etas_es58x/es581_4.c
create mode 100644 drivers/net/can/usb/etas_es58x/es581_4.h
create mode 100644 drivers/net/can/usb/etas_es58x/es58x_core.c
create mode 100644 drivers/net/can/usb/etas_es58x/es58x_core.h
create mode 100644 drivers/net/can/usb/etas_es58x/es58x_fd.c
create mode 100644 drivers/net/can/usb/etas_es58x/es58x_fd.h
--
2.26.2
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-09-26 17:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-26 17:29 [PATCH 0/6] can: add support for ETAS ES58X CAN USB Vincent Mailhol
2020-09-26 17:41 ` [PATCH 1/6] can: dev: can_get_echo_skb(): prevent call to kfree_skb() in hard IRQ context Vincent Mailhol
2020-09-26 17:45 [PATCH 0/6] can: add support for ETAS ES58X CAN USB Vincent Mailhol
2020-09-26 17:57 Vincent Mailhol
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.