From c63845609c4700488e5eacd6ab4d06d5d420e5ef Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Mon, 24 Jun 2019 08:34:13 +0000 Subject: [PATCH 1/7] can: dev: call netif_carrier_off() in register_candev() CONFIG_CAN_LEDS is deprecated. When trying to use the generic netdev trigger as suggested, there's a small inconsistency with the link property: The LED is on initially, stays on when the device is brought up, and then turns off (as expected) when the device is brought down. Make sure the LED always reflects the state of the CAN device. Signed-off-by: Rasmus Villemoes Acked-by: Willem de Bruijn Signed-off-by: Marc Kleine-Budde --- drivers/net/can/dev.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c index b6b93a2d93a5..483d270664cc 100644 --- a/drivers/net/can/dev.c +++ b/drivers/net/can/dev.c @@ -1249,6 +1249,8 @@ int register_candev(struct net_device *dev) return -EINVAL; dev->rtnl_link_ops = &can_link_ops; + netif_carrier_off(dev); + return register_netdev(dev); } EXPORT_SYMBOL_GPL(register_candev); From d4b890aec4bea7334ca2ca56fd3b12fb48a00cd1 Mon Sep 17 00:00:00 2001 From: Nikita Yushchenko Date: Wed, 26 Jun 2019 16:08:48 +0300 Subject: [PATCH 2/7] can: rcar_canfd: fix possible IRQ storm on high load We have observed rcar_canfd driver entering IRQ storm under high load, with following scenario: - rcar_canfd_global_interrupt() in entered due to Rx available, - napi_schedule_prep() is called, and sets NAPIF_STATE_SCHED in state - Rx fifo interrupts are masked, - rcar_canfd_global_interrupt() is entered again, this time due to error interrupt (e.g. due to overflow), - since scheduled napi poller has not yet executed, condition for calling napi_schedule_prep() from rcar_canfd_global_interrupt() remains true, thus napi_schedule_prep() gets called and sets NAPIF_STATE_MISSED flag in state, - later, napi poller function rcar_canfd_rx_poll() gets executed, and calls napi_complete_done(), - due to NAPIF_STATE_MISSED flag in state, this call does not clear NAPIF_STATE_SCHED flag from state, - on return from napi_complete_done(), rcar_canfd_rx_poll() unmasks Rx interrutps, - Rx interrupt happens, rcar_canfd_global_interrupt() gets called and calls napi_schedule_prep(), - since NAPIF_STATE_SCHED is set in state at this time, this call returns false, - due to that false return, rcar_canfd_global_interrupt() returns without masking Rx interrupt - and this results into IRQ storm: unmasked Rx interrupt happens again and again is misprocessed in the same way. This patch fixes that scenario by unmasking Rx interrupts only when napi_complete_done() returns true, which means it has cleared NAPIF_STATE_SCHED in state. Fixes: dd3bd23eb438 ("can: rcar_canfd: Add Renesas R-Car CAN FD driver") Signed-off-by: Nikita Yushchenko Cc: linux-stable Signed-off-by: Marc Kleine-Budde --- drivers/net/can/rcar/rcar_canfd.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c index 05410008aa6b..de34a4b82d4a 100644 --- a/drivers/net/can/rcar/rcar_canfd.c +++ b/drivers/net/can/rcar/rcar_canfd.c @@ -1508,10 +1508,11 @@ static int rcar_canfd_rx_poll(struct napi_struct *napi, int quota) /* All packets processed */ if (num_pkts < quota) { - napi_complete_done(napi, num_pkts); - /* Enable Rx FIFO interrupts */ - rcar_canfd_set_bit(priv->base, RCANFD_RFCC(ridx), - RCANFD_RFCC_RFIE); + if (napi_complete_done(napi, num_pkts)) { + /* Enable Rx FIFO interrupts */ + rcar_canfd_set_bit(priv->base, RCANFD_RFCC(ridx), + RCANFD_RFCC_RFIE); + } } return num_pkts; } From 375f755899b8fc21196197e02aab26257df26e85 Mon Sep 17 00:00:00 2001 From: Weitao Hou Date: Tue, 25 Jun 2019 20:50:48 +0800 Subject: [PATCH 3/7] can: mcp251x: add error check when wq alloc failed add error check when workqueue alloc failed, and remove redundant code to make it clear. Fixes: e0000163e30e ("can: Driver for the Microchip MCP251x SPI CAN controllers") Signed-off-by: Weitao Hou Acked-by: Willem de Bruijn Tested-by: Sean Nyekjaer Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/mcp251x.c | 49 ++++++++++++++++------------------- 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c index 44e99e3d7134..2aec934fab0c 100644 --- a/drivers/net/can/spi/mcp251x.c +++ b/drivers/net/can/spi/mcp251x.c @@ -664,17 +664,6 @@ static int mcp251x_power_enable(struct regulator *reg, int enable) return regulator_disable(reg); } -static void mcp251x_open_clean(struct net_device *net) -{ - struct mcp251x_priv *priv = netdev_priv(net); - struct spi_device *spi = priv->spi; - - free_irq(spi->irq, priv); - mcp251x_hw_sleep(spi); - mcp251x_power_enable(priv->transceiver, 0); - close_candev(net); -} - static int mcp251x_stop(struct net_device *net) { struct mcp251x_priv *priv = netdev_priv(net); @@ -940,37 +929,43 @@ static int mcp251x_open(struct net_device *net) flags | IRQF_ONESHOT, DEVICE_NAME, priv); if (ret) { dev_err(&spi->dev, "failed to acquire irq %d\n", spi->irq); - mcp251x_power_enable(priv->transceiver, 0); - close_candev(net); - goto open_unlock; + goto out_close; } priv->wq = alloc_workqueue("mcp251x_wq", WQ_FREEZABLE | WQ_MEM_RECLAIM, 0); + if (!priv->wq) { + ret = -ENOMEM; + goto out_clean; + } INIT_WORK(&priv->tx_work, mcp251x_tx_work_handler); INIT_WORK(&priv->restart_work, mcp251x_restart_work_handler); ret = mcp251x_hw_reset(spi); - if (ret) { - mcp251x_open_clean(net); - goto open_unlock; - } + if (ret) + goto out_free_wq; ret = mcp251x_setup(net, spi); - if (ret) { - mcp251x_open_clean(net); - goto open_unlock; - } + if (ret) + goto out_free_wq; ret = mcp251x_set_normal_mode(spi); - if (ret) { - mcp251x_open_clean(net); - goto open_unlock; - } + if (ret) + goto out_free_wq; can_led_event(net, CAN_LED_EVENT_OPEN); netif_wake_queue(net); + mutex_unlock(&priv->mcp_lock); -open_unlock: + return 0; + +out_free_wq: + destroy_workqueue(priv->wq); +out_clean: + free_irq(spi->irq, priv); + mcp251x_hw_sleep(spi); +out_close: + mcp251x_power_enable(priv->transceiver, 0); + close_candev(net); mutex_unlock(&priv->mcp_lock); return ret; } From e9f2a856e102fa27715b94bcc2240f686536d29b Mon Sep 17 00:00:00 2001 From: Wen Yang Date: Sat, 6 Jul 2019 11:37:20 +0800 Subject: [PATCH 4/7] can: flexcan: fix an use-after-free in flexcan_setup_stop_mode() The gpr_np variable is still being used in dev_dbg() after the of_node_put() call, which may result in use-after-free. Fixes: de3578c198c6 ("can: flexcan: add self wakeup support") Signed-off-by: Wen Yang Cc: linux-stable # >= v5.0 Signed-off-by: Marc Kleine-Budde --- drivers/net/can/flexcan.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index f2fe344593d5..33ce45d51e15 100644 --- a/drivers/net/can/flexcan.c +++ b/drivers/net/can/flexcan.c @@ -1437,10 +1437,10 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev) priv = netdev_priv(dev); priv->stm.gpr = syscon_node_to_regmap(gpr_np); - of_node_put(gpr_np); if (IS_ERR(priv->stm.gpr)) { dev_dbg(&pdev->dev, "could not find gpr regmap\n"); - return PTR_ERR(priv->stm.gpr); + ret = PTR_ERR(priv->stm.gpr); + goto out_put_node; } priv->stm.req_gpr = out_val[1]; @@ -1455,7 +1455,9 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev) device_set_wakeup_capable(&pdev->dev, true); - return 0; +out_put_node: + of_node_put(gpr_np); + return ret; } static const struct of_device_id flexcan_of_match[] = { From 5f186c257fa4808bb7f14e643b9fba3e11f08a30 Mon Sep 17 00:00:00 2001 From: Joakim Zhang Date: Tue, 2 Jul 2019 01:45:41 +0000 Subject: [PATCH 5/7] can: flexcan: fix stop mode acknowledgment To enter stop mode, the CPU should manually assert a global Stop Mode request and check the acknowledgment asserted by FlexCAN. The CPU must only consider the FlexCAN in stop mode when both request and acknowledgment conditions are satisfied. Fixes: de3578c198c6 ("can: flexcan: add self wakeup support") Reported-by: Marc Kleine-Budde Signed-off-by: Joakim Zhang Cc: linux-stable # >= v5.0 Signed-off-by: Marc Kleine-Budde --- drivers/net/can/flexcan.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index 33ce45d51e15..fcec8bcb53d6 100644 --- a/drivers/net/can/flexcan.c +++ b/drivers/net/can/flexcan.c @@ -400,9 +400,10 @@ static void flexcan_enable_wakeup_irq(struct flexcan_priv *priv, bool enable) priv->write(reg_mcr, ®s->mcr); } -static inline void flexcan_enter_stop_mode(struct flexcan_priv *priv) +static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv) { struct flexcan_regs __iomem *regs = priv->regs; + unsigned int ackval; u32 reg_mcr; reg_mcr = priv->read(®s->mcr); @@ -412,20 +413,37 @@ static inline void flexcan_enter_stop_mode(struct flexcan_priv *priv) /* enable stop request */ regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr, 1 << priv->stm.req_bit, 1 << priv->stm.req_bit); + + /* get stop acknowledgment */ + if (regmap_read_poll_timeout(priv->stm.gpr, priv->stm.ack_gpr, + ackval, ackval & (1 << priv->stm.ack_bit), + 0, FLEXCAN_TIMEOUT_US)) + return -ETIMEDOUT; + + return 0; } -static inline void flexcan_exit_stop_mode(struct flexcan_priv *priv) +static inline int flexcan_exit_stop_mode(struct flexcan_priv *priv) { struct flexcan_regs __iomem *regs = priv->regs; + unsigned int ackval; u32 reg_mcr; /* remove stop request */ regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr, 1 << priv->stm.req_bit, 0); + /* get stop acknowledgment */ + if (regmap_read_poll_timeout(priv->stm.gpr, priv->stm.ack_gpr, + ackval, !(ackval & (1 << priv->stm.ack_bit)), + 0, FLEXCAN_TIMEOUT_US)) + return -ETIMEDOUT; + reg_mcr = priv->read(®s->mcr); reg_mcr &= ~FLEXCAN_MCR_SLF_WAK; priv->write(reg_mcr, ®s->mcr); + + return 0; } static inline void flexcan_error_irq_enable(const struct flexcan_priv *priv) @@ -1614,7 +1632,9 @@ static int __maybe_unused flexcan_suspend(struct device *device) */ if (device_may_wakeup(device)) { enable_irq_wake(dev->irq); - flexcan_enter_stop_mode(priv); + err = flexcan_enter_stop_mode(priv); + if (err) + return err; } else { err = flexcan_chip_disable(priv); if (err) @@ -1664,10 +1684,13 @@ static int __maybe_unused flexcan_noirq_resume(struct device *device) { struct net_device *dev = dev_get_drvdata(device); struct flexcan_priv *priv = netdev_priv(dev); + int err; if (netif_running(dev) && device_may_wakeup(device)) { flexcan_enable_wakeup_irq(priv, false); - flexcan_exit_stop_mode(priv); + err = flexcan_exit_stop_mode(priv); + if (err) + return err; } return 0; From fee6a8923ae0d318a7f7950c6c6c28a96cea099b Mon Sep 17 00:00:00 2001 From: Stephane Grosjean Date: Fri, 5 Jul 2019 15:32:16 +0200 Subject: [PATCH 6/7] can: peak_usb: fix potential double kfree_skb() When closing the CAN device while tx skbs are inflight, echo skb could be released twice. By calling close_candev() before unlinking all pending tx urbs, then the internal echo_skb[] array is fully and correctly cleared before the USB write callback and, therefore, can_get_echo_skb() are called, for each aborted URB. Fixes: bb4785551f64 ("can: usb: PEAK-System Technik USB adapters driver core") Signed-off-by: Stephane Grosjean Cc: linux-stable Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/peak_usb/pcan_usb_core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c index 458154c9b482..22b9c8e6d040 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c @@ -568,16 +568,16 @@ static int peak_usb_ndo_stop(struct net_device *netdev) dev->state &= ~PCAN_USB_STATE_STARTED; netif_stop_queue(netdev); + close_candev(netdev); + + dev->can.state = CAN_STATE_STOPPED; + /* unlink all pending urbs and free used memory */ peak_usb_unlink_all_urbs(dev); if (dev->adapter->dev_stop) dev->adapter->dev_stop(dev); - close_candev(netdev); - - dev->can.state = CAN_STATE_STOPPED; - /* can set bus off now */ if (dev->adapter->dev_set_bus) { int err = dev->adapter->dev_set_bus(dev, 0); From b7a14297f102b6e2ce6f16feffebbb9bde1e9b55 Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Sat, 18 May 2019 17:35:43 +0800 Subject: [PATCH 7/7] can: gw: Fix error path of cgw_module_init This patch add error path for cgw_module_init to avoid possible crash if some error occurs. Fixes: c1aabdf379bc ("can-gw: add netlink based CAN routing") Signed-off-by: YueHaibing Acked-by: Oliver Hartkopp Signed-off-by: Marc Kleine-Budde --- net/can/gw.c | 46 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/net/can/gw.c b/net/can/gw.c index 5275ddf580bc..72711053ebe6 100644 --- a/net/can/gw.c +++ b/net/can/gw.c @@ -1046,32 +1046,50 @@ static __init int cgw_module_init(void) pr_info("can: netlink gateway (rev " CAN_GW_VERSION ") max_hops=%d\n", max_hops); - register_pernet_subsys(&cangw_pernet_ops); + ret = register_pernet_subsys(&cangw_pernet_ops); + if (ret) + return ret; + + ret = -ENOMEM; cgw_cache = kmem_cache_create("can_gw", sizeof(struct cgw_job), 0, 0, NULL); - if (!cgw_cache) - return -ENOMEM; + goto out_cache_create; /* set notifier */ notifier.notifier_call = cgw_notifier; - register_netdevice_notifier(¬ifier); + ret = register_netdevice_notifier(¬ifier); + if (ret) + goto out_register_notifier; ret = rtnl_register_module(THIS_MODULE, PF_CAN, RTM_GETROUTE, NULL, cgw_dump_jobs, 0); - if (ret) { - unregister_netdevice_notifier(¬ifier); - kmem_cache_destroy(cgw_cache); - return -ENOBUFS; - } + if (ret) + goto out_rtnl_register1; - /* Only the first call to rtnl_register_module can fail */ - rtnl_register_module(THIS_MODULE, PF_CAN, RTM_NEWROUTE, - cgw_create_job, NULL, 0); - rtnl_register_module(THIS_MODULE, PF_CAN, RTM_DELROUTE, - cgw_remove_job, NULL, 0); + ret = rtnl_register_module(THIS_MODULE, PF_CAN, RTM_NEWROUTE, + cgw_create_job, NULL, 0); + if (ret) + goto out_rtnl_register2; + ret = rtnl_register_module(THIS_MODULE, PF_CAN, RTM_DELROUTE, + cgw_remove_job, NULL, 0); + if (ret) + goto out_rtnl_register3; return 0; + +out_rtnl_register3: + rtnl_unregister(PF_CAN, RTM_NEWROUTE); +out_rtnl_register2: + rtnl_unregister(PF_CAN, RTM_GETROUTE); +out_rtnl_register1: + unregister_netdevice_notifier(¬ifier); +out_register_notifier: + kmem_cache_destroy(cgw_cache); +out_cache_create: + unregister_pernet_subsys(&cangw_pernet_ops); + + return ret; } static __exit void cgw_module_exit(void)