The RX callback can process another OPEN command after the removal path has cancelled register_netdev_work, allowing a late registration to outlive the devm-managed DMUX. It can also inspect netdev state while those netdevs are being unregistered. PC-ack remains live past the final PC shutdown. Set a removal gate under the RX callback lock, then permanently disable and drain registration work before unregistering interfaces. Continue to recycle RX buffers until normal power shutdown so ndo_stop can still send its CLOSE command. Drain PC-ack as well as PC before the final power cleanup. This separates the removal fix from the normal power- control state change. Found by the author's in-house static analysis tool. Fixes: 21a0ffd9b38c ("net: wwan: Add Qualcomm BAM-DMUX WWAN network driver") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/all/30dc9ad2-0ff7-4d97-8b01-ab2267fb56b8@redhat.com/ Signed-off-by: Hongyan Xu Signed-off-by: Zehao Gao --- Discovery: found by the author's in-house static analysis tool. drivers/net/wwan/qcom_bam_dmux.c | 102 ++++++++++++------------------- 1 file changed, 40 insertions(+), 62 deletions(-) diff --git a/drivers/net/wwan/qcom_bam_dmux.c b/drivers/net/wwan/qcom_bam_dmux.c index 48ad1bc..b83cb51 100644 --- a/drivers/net/wwan/qcom_bam_dmux.c +++ b/drivers/net/wwan/qcom_bam_dmux.c @@ -88,6 +88,7 @@ struct bam_dmux { struct dma_chan *rx, *tx; spinlock_t rx_lock; /* Protect RX callbacks against shutdown */ bool rx_active; + bool removing; /* Protected by rx_lock */ struct bam_dmux_skb_dma rx_skbs[BAM_DMUX_NUM_SKB]; struct bam_dmux_skb_dma tx_skbs[BAM_DMUX_NUM_SKB]; spinlock_t tx_lock; /* Protect tx_skbs, tx_next_skb */ @@ -491,65 +492,25 @@ static void bam_dmux_register_netdev_work(struct work_struct *work) } } -static void bam_dmux_rx_callback(void *data) +static void bam_dmux_rx_callback(void *data); + +static bool bam_dmux_skb_dma_submit_rx(struct bam_dmux_skb_dma *skb_dma) { - struct bam_dmux_skb_dma *skb_dma = data; struct bam_dmux *dmux = skb_dma->dmux; - struct bam_dmux_hdr *hdr; - struct sk_buff *skb; - unsigned long flags; - - spin_lock_irqsave(&dmux->rx_lock, flags); - if (!dmux->rx_active) { - spin_unlock_irqrestore(&dmux->rx_lock, flags); - return; - } - - skb = skb_dma->skb; - hdr = (struct bam_dmux_hdr *)skb->data; - bam_dmux_skb_dma_unmap(skb_dma, DMA_FROM_DEVICE); - if (dmux->removing) { - spin_unlock_irqrestore(&dmux->rx_lock, flags); - goto requeue; - } - spin_unlock_irqrestore(&dmux->rx_lock, flags); - - if (hdr->magic != BAM_DMUX_HDR_MAGIC) { - dev_err(dmux->dev, "Invalid magic in header: %#x\n", hdr->magic); - goto requeue; - } - - if (hdr->ch >= BAM_DMUX_NUM_CH) { - dev_dbg(dmux->dev, "Unsupported channel: %u\n", hdr->ch); - goto requeue; - } + struct dma_async_tx_descriptor *desc; - switch (hdr->cmd) { - case BAM_DMUX_CMD_DATA: - bam_dmux_cmd_data(skb_dma); - break; - case BAM_DMUX_CMD_OPEN: - bam_dmux_cmd_open(dmux, hdr); - break; - case BAM_DMUX_CMD_CLOSE: - bam_dmux_cmd_close(dmux, hdr); - break; - default: - dev_err(dmux->dev, "Unsupported command %u on channel %u\n", - hdr->cmd, hdr->ch); - break; + desc = dmaengine_prep_slave_single(dmux->rx, skb_dma->addr, + skb_dma->skb->len, DMA_DEV_TO_MEM, + DMA_PREP_INTERRUPT); + if (!desc) { + dev_err(dmux->dev, "Failed to prepare RX DMA buffer\n"); + return false; } -requeue: - /* - * Recheck the gate: power-off may have stopped refill while the - * command was dispatched without the lock. - */ - spin_lock_irqsave(&dmux->rx_lock, flags); - if (dmux->rx_active && - bam_dmux_skb_dma_queue_rx(skb_dma, GFP_ATOMIC)) - dma_async_issue_pending(dmux->rx); - spin_unlock_irqrestore(&dmux->rx_lock, flags); + desc->callback = bam_dmux_rx_callback; + desc->callback_param = skb_dma; + desc->cookie = dmaengine_submit(desc); + return true; } static bool bam_dmux_skb_dma_queue_rx(struct bam_dmux_skb_dma *skb_dma, gfp_t gfp) @@ -648,21 +609,28 @@ static void bam_dmux_rx_callback(void *data) unsigned long flags; spin_lock_irqsave(&dmux->rx_lock, flags); - if (!dmux->rx_active) - goto out_unlock; + if (!dmux->rx_active) { + spin_unlock_irqrestore(&dmux->rx_lock, flags); + return; + } skb = skb_dma->skb; hdr = (struct bam_dmux_hdr *)skb->data; bam_dmux_skb_dma_unmap(skb_dma, DMA_FROM_DEVICE); + if (dmux->removing) { + spin_unlock_irqrestore(&dmux->rx_lock, flags); + goto requeue; + } + spin_unlock_irqrestore(&dmux->rx_lock, flags); if (hdr->magic != BAM_DMUX_HDR_MAGIC) { dev_err(dmux->dev, "Invalid magic in header: %#x\n", hdr->magic); - goto out; + goto requeue; } if (hdr->ch >= BAM_DMUX_NUM_CH) { dev_dbg(dmux->dev, "Unsupported channel: %u\n", hdr->ch); - goto out; + goto requeue; } switch (hdr->cmd) { @@ -681,10 +649,15 @@ static void bam_dmux_rx_callback(void *data) break; } -out: - if (bam_dmux_skb_dma_queue_rx(skb_dma, GFP_ATOMIC)) +requeue: + /* + * Recheck the gate: power-off may have stopped refill while the + * command was dispatched without the lock. + */ + spin_lock_irqsave(&dmux->rx_lock, flags); + if (dmux->rx_active && + bam_dmux_skb_dma_queue_rx(skb_dma, GFP_ATOMIC)) dma_async_issue_pending(dmux->rx); -out_unlock: spin_unlock_irqrestore(&dmux->rx_lock, flags); } @@ -1077,8 +1050,12 @@ static void bam_dmux_remove(struct platform_device *pdev) LIST_HEAD(list); int i; + /* Stop RX publication before unregistering any network interface. */ + scoped_guard(spinlock_irqsave, &dmux->rx_lock) + dmux->removing = true; + disable_work_sync(&dmux->register_netdev_work); + /* Unregister network interfaces */ - cancel_work_sync(&dmux->register_netdev_work); rtnl_lock(); for (i = 0; i < BAM_DMUX_NUM_CH; ++i) if (dmux->netdevs[i]) @@ -1099,6 +1076,7 @@ static void bam_dmux_remove(struct platform_device *pdev) dev_err(dev, "Timed out waiting for remote side to suspend\n"); /* Make sure everything is cleaned up before we return */ + disable_irq(dmux->pc_ack_irq); disable_irq(dmux->pc_irq); mutex_lock(&dmux->power_lock); bam_dmux_power_off(dmux); -- 2.50.1.windows.1