From: Marc Kleine-Budde <mkl@pengutronix.de> Date: 2021-03-01 11:22:12
Hello Jakub, hello David,
this is a pull request of 6 patches for net/master.
The first 3 patches are by Joakim Zhang for the flexcan driver and fix
the probing and starting of the chip.
The next patch is by me, for the mcp251xfd driver and reverts the BQL
support. BQL support got mainline with rc1 and assumes that CAN frames
are always echoed, which is not the case. A proper fix requires
changes more changes and will be rolled out via linux-can-next later.
Oleksij Rempel's patch fixes the socket ref counting if socket was
closed before setting skb ownership.
Torin Cooper-Bennun's patch for the tcan4x5x driver fixes a race
condition, where the chip is first attached the bus and then the MRAM
is initialized, which may result in lost data.
regards,
Marc
---
The following changes since commit 447621e373bd1b22300445639b43c39f399e4c73:
Merge branch 'net-hns3-fixes-fot-net' (2021-02-28 12:04:02 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-5.12-20210301
for you to fetch changes up to 2712625200ed69c642b9abc3a403830c4643364c:
can: tcan4x5x: tcan4x5x_init(): fix initialization - clear MRAM before entering Normal Mode (2021-03-01 11:45:15 +0100)
----------------------------------------------------------------
linux-can-fixes-for-5.12-20210301
----------------------------------------------------------------
Joakim Zhang (3):
can: flexcan: assert FRZ bit in flexcan_chip_freeze()
can: flexcan: enable RX FIFO after FRZ/HALT valid
can: flexcan: invoke flexcan_chip_freeze() to enter freeze mode
Marc Kleine-Budde (1):
can: mcp251xfd: revert "can: mcp251xfd: add BQL support"
Oleksij Rempel (1):
can: skb: can_skb_set_owner(): fix ref counting if socket was closed before setting skb ownership
Torin Cooper-Bennun (1):
can: tcan4x5x: tcan4x5x_init(): fix initialization - clear MRAM before entering Normal Mode
drivers/net/can/flexcan.c | 24 +++++++++++++++---------
drivers/net/can/m_can/tcan4x5x-core.c | 6 +++---
drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 21 ++++-----------------
include/linux/can/skb.h | 8 ++++++--
4 files changed, 28 insertions(+), 31 deletions(-)
From: Marc Kleine-Budde <mkl@pengutronix.de> Date: 2021-03-01 11:22:12
From: Joakim Zhang <redacted>
Assert HALT bit to enter freeze mode, there is a premise that FRZ bit is
asserted. This patch asserts FRZ bit in flexcan_chip_freeze, although
the reset value is 1b'1. This is a prepare patch, later patch will
invoke flexcan_chip_freeze() to enter freeze mode, which polling freeze
mode acknowledge.
Fixes: b1aa1c7a2165b ("can: flexcan: fix transition from and to freeze mode in chip_{,un}freeze")
Link: https://lore.kernel.org/r/20210218110037.16591-2-qiangqing.zhang@nxp.com
Signed-off-by: Joakim Zhang <redacted>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/flexcan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -1865,10 +1865,14 @@ static int register_flexcandev(struct net_device *dev)if(err)gotoout_chip_disable;-/* set freeze, halt and activate FIFO, restrict register access */+/* set freeze, halt */+err=flexcan_chip_freeze(priv);+if(err)+gotoout_chip_disable;++/* activate FIFO, restrict register access */reg=priv->read(®s->mcr);-reg|=FLEXCAN_MCR_FRZ|FLEXCAN_MCR_HALT|-FLEXCAN_MCR_FEN|FLEXCAN_MCR_SUPV;+reg|=FLEXCAN_MCR_FEN|FLEXCAN_MCR_SUPV;priv->write(reg,®s->mcr);/* Currently we only support newer versions of this core
@@ -1480,10 +1480,13 @@ static int flexcan_chip_start(struct net_device *dev)flexcan_set_bittiming(dev);+/* set freeze, halt */+err=flexcan_chip_freeze(priv);+if(err)+gotoout_chip_disable;+/* MCR*-*enablefreeze-*haltnow*onlysupervisoraccess*enablewarningint*enableindividualRXmasking
@@ -1492,9 +1495,8 @@ static int flexcan_chip_start(struct net_device *dev)*/reg_mcr=priv->read(®s->mcr);reg_mcr&=~FLEXCAN_MCR_MAXMB(0xff);-reg_mcr|=FLEXCAN_MCR_FRZ|FLEXCAN_MCR_HALT|FLEXCAN_MCR_SUPV|-FLEXCAN_MCR_WRN_EN|FLEXCAN_MCR_IRMQ|FLEXCAN_MCR_IDAM_C|-FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);+reg_mcr|=FLEXCAN_MCR_SUPV|FLEXCAN_MCR_WRN_EN|FLEXCAN_MCR_IRMQ|+FLEXCAN_MCR_IDAM_C|FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);/* MCR*
@@ -1480,10 +1480,13 @@ static int flexcan_chip_start(struct net_device *dev)flexcan_set_bittiming(dev);+/* set freeze, halt */+err=flexcan_chip_freeze(priv);+if(err)+gotoout_chip_disable;
With v5.12-rc3, both my FlexCAN controllers on an i.MX6Q now divide by zero
on probe because priv->can.bittiming.bitrate == 0 inside of flexcan_chip_freeze.
Reverting this patch fixes it.
quoted hunk
+
/* MCR
*
- * enable freeze
- * halt now
* only supervisor access
* enable warning int
* enable individual RX masking
@@ -1480,10 +1480,13 @@ static int flexcan_chip_start(struct net_device *dev)flexcan_set_bittiming(dev);+/* set freeze, halt */+err=flexcan_chip_freeze(priv);+if(err)+gotoout_chip_disable;
With v5.12-rc3, both my FlexCAN controllers on an i.MX6Q now divide by zero
on probe because priv->can.bittiming.bitrate == 0 inside of flexcan_chip_freeze.
Reverting this patch fixes it.
From: Marc Kleine-Budde <mkl@pengutronix.de> Date: 2021-03-01 11:23:33
In the following 4 patches
| 99842c9685ab can: dev: can_rx_offload_get_echo_skb(): extend to return can frame length
| 9420e1d495e2 can: dev: can_get_echo_skb(): extend to return can frame length
| 1dcb6e57db83 can: dev: can_put_echo_skb(): extend to handle frame_len
| f0ef72febc9a can: dev: extend struct can_skb_priv to hold CAN frame length
the CAN echo SKB support was extended to hold the CAN frame
length (which is the length of the CAN frame on the wire). It is meant
as a helper for BQL support, to avoid the re-calculation of the frame
length before sending it and on TX-completion.
However if the CAN frame is send without the request to be looped back
the SKB is discarded in can_put_echo_skb() and the subsequent
can_get_echo_skb() and can_rx_offload_get_echo_skb() return 0 for the
CAN frame length. This results in BQL stalling the TX queue after a
few packages.
Until the BQL helpers can_get_echo_skb() and
can_rx_offload_get_echo_skb() are fixed, revert the BQL support for
the mcp251xfd driver.
This reverts commit 4162e18e949ba520d5116ac0323500355479a00e.
Fixes: 4162e18e949b ("can: mcp251xfd: add BQL support")
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Thomas Kopp <thomas.kopp@microchip.com>
Link: https://lore.kernel.org/r/20210228083347.28580-1-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
.../net/can/spi/mcp251xfd/mcp251xfd-core.c | 21 ++++---------------
1 file changed, 4 insertions(+), 17 deletions(-)
@@ -1353,9 +1348,7 @@ static int mcp251xfd_handle_tefif(struct mcp251xfd_priv *priv)}for(i=0;i<len;i++){-unsignedintframe_len;--err=mcp251xfd_handle_tefif_one(priv,&hw_tef_obj[i],&frame_len);+err=mcp251xfd_handle_tefif_one(priv,&hw_tef_obj[i]);/* -EAGAIN means the Sequence Number in the TEF*doesn'tmatchourtef_tail.Thiscanhappenifwe*readtheTEFobjectstooearly.Leaveloopletthe
@@ -1365,8 +1358,6 @@ static int mcp251xfd_handle_tefif(struct mcp251xfd_priv *priv)gotoout_netif_wake_queue;if(err)returnerr;--total_frame_len+=frame_len;}out_netif_wake_queue:
@@ -1397,7 +1388,6 @@ static int mcp251xfd_handle_tefif(struct mcp251xfd_priv *priv)returnerr;tx_ring->tail+=len;-netdev_completed_queue(priv->ndev,len,total_frame_len);err=mcp251xfd_check_tef_tail(priv);if(err)
From: Marc Kleine-Budde <mkl@pengutronix.de> Date: 2021-03-01 11:23:34
From: Torin Cooper-Bennun <redacted>
This patch prevents a potentially destructive race condition. The
device is fully operational on the bus after entering Normal Mode, so
zeroing the MRAM after entering this mode may lead to loss of
information, e.g. new received messages.
This patch fixes the problem by first initializing the MRAM, then
bringing the device into Normale Mode.
Fixes: 5443c226ba91 ("can: tcan4x5x: Add tcan4x5x driver to the kernel")
Link: https://lore.kernel.org/r/20210226163440.313628-1-torin@maxiluxsystems.com
Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Torin Cooper-Bennun <redacted>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/m_can/tcan4x5x-core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
@@ -237,14 +237,14 @@ static int tcan4x5x_init(struct m_can_classdev *cdev)if(ret)returnret;+/* Zero out the MCAN buffers */+m_can_init_ram(cdev);+ret=regmap_update_bits(tcan4x5x->regmap,TCAN4X5X_CONFIG,TCAN4X5X_MODE_SEL_MASK,TCAN4X5X_MODE_NORMAL);if(ret)returnret;-/* Zero out the MCAN buffers */-m_can_init_ram(cdev);-returnret;}
From: Marc Kleine-Budde <mkl@pengutronix.de> Date: 2021-03-01 11:23:56
From: Oleksij Rempel <o.rempel@pengutronix.de>
There are two ref count variables controlling the free()ing of a socket:
- struct sock::sk_refcnt - which is changed by sock_hold()/sock_put()
- struct sock::sk_wmem_alloc - which accounts the memory allocated by
the skbs in the send path.
In case there are still TX skbs on the fly and the socket() is closed,
the struct sock::sk_refcnt reaches 0. In the TX-path the CAN stack
clones an "echo" skb, calls sock_hold() on the original socket and
references it. This produces the following back trace:
| WARNING: CPU: 0 PID: 280 at lib/refcount.c:25 refcount_warn_saturate+0x114/0x134
| refcount_t: addition on 0; use-after-free.
| Modules linked in: coda_vpu(E) v4l2_jpeg(E) videobuf2_vmalloc(E) imx_vdoa(E)
| CPU: 0 PID: 280 Comm: test_can.sh Tainted: G E 5.11.0-04577-gf8ff6603c617 #203
| Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
| Backtrace:
| [<80bafea4>] (dump_backtrace) from [<80bb0280>] (show_stack+0x20/0x24) r7:00000000 r6:600f0113 r5:00000000 r4:81441220
| [<80bb0260>] (show_stack) from [<80bb593c>] (dump_stack+0xa0/0xc8)
| [<80bb589c>] (dump_stack) from [<8012b268>] (__warn+0xd4/0x114) r9:00000019 r8:80f4a8c2 r7:83e4150c r6:00000000 r5:00000009 r4:80528f90
| [<8012b194>] (__warn) from [<80bb09c4>] (warn_slowpath_fmt+0x88/0xc8) r9:83f26400 r8:80f4a8d1 r7:00000009 r6:80528f90 r5:00000019 r4:80f4a8c2
| [<80bb0940>] (warn_slowpath_fmt) from [<80528f90>] (refcount_warn_saturate+0x114/0x134) r8:00000000 r7:00000000 r6:82b44000 r5:834e5600 r4:83f4d540
| [<80528e7c>] (refcount_warn_saturate) from [<8079a4c8>] (__refcount_add.constprop.0+0x4c/0x50)
| [<8079a47c>] (__refcount_add.constprop.0) from [<8079a57c>] (can_put_echo_skb+0xb0/0x13c)
| [<8079a4cc>] (can_put_echo_skb) from [<8079ba98>] (flexcan_start_xmit+0x1c4/0x230) r9:00000010 r8:83f48610 r7:0fdc0000 r6:0c080000 r5:82b44000 r4:834e5600
| [<8079b8d4>] (flexcan_start_xmit) from [<80969078>] (netdev_start_xmit+0x44/0x70) r9:814c0ba0 r8:80c8790c r7:00000000 r6:834e5600 r5:82b44000 r4:82ab1f00
| [<80969034>] (netdev_start_xmit) from [<809725a4>] (dev_hard_start_xmit+0x19c/0x318) r9:814c0ba0 r8:00000000 r7:82ab1f00 r6:82b44000 r5:00000000 r4:834e5600
| [<80972408>] (dev_hard_start_xmit) from [<809c6584>] (sch_direct_xmit+0xcc/0x264) r10:834e5600 r9:00000000 r8:00000000 r7:82b44000 r6:82ab1f00 r5:834e5600 r4:83f27400
| [<809c64b8>] (sch_direct_xmit) from [<809c6c0c>] (__qdisc_run+0x4f0/0x534)
To fix this problem, only set skb ownership to sockets which have still
a ref count > 0.
Fixes: 0ae89beb283a ("can: add destructor for self generated skbs")
Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Andre Naujoks <redacted>
Link: https://lore.kernel.org/r/20210226092456.27126-1-o.rempel@pengutronix.de
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
include/linux/can/skb.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
@@ -65,8 +65,12 @@ static inline void can_skb_reserve(struct sk_buff *skb)staticinlinevoidcan_skb_set_owner(structsk_buff*skb,structsock*sk){-if(sk){-sock_hold(sk);+/* If the socket has already been closed by user space, the+*refcountmayalreadybe0(andthesocketwillbefreed+*afterthelastTXskbhasbeenfreed).Soonlyincrease+*socketrefcountiftherefcountis>0.+*/+if(sk&&refcount_inc_not_zero(&sk->sk_refcnt)){skb->destructor=sock_efree;skb->sk=sk;}