| Message ID | 20221212084139.3277913-4-yangyingliang@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1232946cf522b8de9e398828bde325d7c41f29dd |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | mISDN: don't call dev_kfree_skb/kfree_skb() under spin_lock_irqsave() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net |
| netdev/fixes_present | success | Fixes tag present in non-next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 9 this patch: 9 |
| netdev/cc_maintainers | success | CCed 3 of 3 maintainers |
| netdev/build_clang | success | Errors and warnings before: 2 this patch: 2 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 9 this patch: 9 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 71 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Mon, 2022-12-12 at 16:41 +0800, Yang Yingliang wrote: > It is not allowed to call kfree_skb() or consume_skb() from hardware > interrupt context or with hardware interrupts being disabled. > > skb_queue_purge() is called under spin_lock_irqsave() in handle_dmsg() > and hfcm_l1callback(), kfree_skb() is called in them, to fix this, use > skb_queue_splice_init() to move the dch->squeue to a free queue, also > enqueue the tx_skb and rx_skb, at last calling __skb_queue_purge() to > free the SKBs afer unlock. > > Fixes: af69fb3a8ffa ("Add mISDN HFC multiport driver") > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > drivers/isdn/hardware/mISDN/hfcmulti.c | 19 +++++++++++++------ > 1 file changed, 13 insertions(+), 6 deletions(-) > > diff --git a/drivers/isdn/hardware/mISDN/hfcmulti.c b/drivers/isdn/hardware/mISDN/hfcmulti.c > index 4f7eaa17fb27..e840609c50eb 100644 > --- a/drivers/isdn/hardware/mISDN/hfcmulti.c > +++ b/drivers/isdn/hardware/mISDN/hfcmulti.c > @@ -3217,6 +3217,7 @@ static int > hfcm_l1callback(struct dchannel *dch, u_int cmd) > { > struct hfc_multi *hc = dch->hw; > + struct sk_buff_head free_queue; > u_long flags; > > switch (cmd) { > @@ -3245,6 +3246,7 @@ hfcm_l1callback(struct dchannel *dch, u_int cmd) > l1_event(dch->l1, HW_POWERUP_IND); > break; > case HW_DEACT_REQ: > + __skb_queue_head_init(&free_queue); > /* start deactivation */ > spin_lock_irqsave(&hc->lock, flags); > if (hc->ctype == HFC_TYPE_E1) { > @@ -3264,20 +3266,21 @@ hfcm_l1callback(struct dchannel *dch, u_int cmd) > plxsd_checksync(hc, 0); > } > } > - skb_queue_purge(&dch->squeue); > + skb_queue_splice_init(&dch->squeue, &free_queue); > if (dch->tx_skb) { > - dev_kfree_skb(dch->tx_skb); > + __skb_queue_tail(&free_queue, dch->tx_skb); > dch->tx_skb = NULL; > } > dch->tx_idx = 0; > if (dch->rx_skb) { > - dev_kfree_skb(dch->rx_skb); > + __skb_queue_tail(&free_queue, dch->rx_skb); > dch->rx_skb = NULL; > } > test_and_clear_bit(FLG_TX_BUSY, &dch->Flags); > if (test_and_clear_bit(FLG_BUSY_TIMER, &dch->Flags)) > del_timer(&dch->timer); > spin_unlock_irqrestore(&hc->lock, flags); > + __skb_queue_purge(&free_queue); > break; > case HW_POWERUP_REQ: > spin_lock_irqsave(&hc->lock, flags); > @@ -3384,6 +3387,9 @@ handle_dmsg(struct mISDNchannel *ch, struct sk_buff *skb) > case PH_DEACTIVATE_REQ: > test_and_clear_bit(FLG_L2_ACTIVATED, &dch->Flags); > if (dch->dev.D.protocol != ISDN_P_TE_S0) { > + struct sk_buff_head free_queue; > + > + __skb_queue_head_init(&free_queue); > spin_lock_irqsave(&hc->lock, flags); > if (debug & DEBUG_HFCMULTI_MSG) > printk(KERN_DEBUG > @@ -3405,14 +3411,14 @@ handle_dmsg(struct mISDNchannel *ch, struct sk_buff *skb) > /* deactivate */ > dch->state = 1; > } > - skb_queue_purge(&dch->squeue); > + skb_queue_splice_init(&dch->squeue, &free_queue); > if (dch->tx_skb) { > - dev_kfree_skb(dch->tx_skb); > + __skb_queue_tail(&free_queue, dch->tx_skb); > dch->tx_skb = NULL; > } > dch->tx_idx = 0; > if (dch->rx_skb) { > - dev_kfree_skb(dch->rx_skb); > + __skb_queue_tail(&free_queue, dch->rx_skb); > dch->rx_skb = NULL; > } > test_and_clear_bit(FLG_TX_BUSY, &dch->Flags); > @@ -3424,6 +3430,7 @@ handle_dmsg(struct mISDNchannel *ch, struct sk_buff *skb) > #endif > ret = 0; > spin_unlock_irqrestore(&hc->lock, flags); > + __skb_queue_purge(&free_queue); > } else > ret = l1_event(dch->l1, hh->prim); > break; Looks good to me. Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
diff --git a/drivers/isdn/hardware/mISDN/hfcmulti.c b/drivers/isdn/hardware/mISDN/hfcmulti.c index 4f7eaa17fb27..e840609c50eb 100644 --- a/drivers/isdn/hardware/mISDN/hfcmulti.c +++ b/drivers/isdn/hardware/mISDN/hfcmulti.c @@ -3217,6 +3217,7 @@ static int hfcm_l1callback(struct dchannel *dch, u_int cmd) { struct hfc_multi *hc = dch->hw; + struct sk_buff_head free_queue; u_long flags; switch (cmd) { @@ -3245,6 +3246,7 @@ hfcm_l1callback(struct dchannel *dch, u_int cmd) l1_event(dch->l1, HW_POWERUP_IND); break; case HW_DEACT_REQ: + __skb_queue_head_init(&free_queue); /* start deactivation */ spin_lock_irqsave(&hc->lock, flags); if (hc->ctype == HFC_TYPE_E1) { @@ -3264,20 +3266,21 @@ hfcm_l1callback(struct dchannel *dch, u_int cmd) plxsd_checksync(hc, 0); } } - skb_queue_purge(&dch->squeue); + skb_queue_splice_init(&dch->squeue, &free_queue); if (dch->tx_skb) { - dev_kfree_skb(dch->tx_skb); + __skb_queue_tail(&free_queue, dch->tx_skb); dch->tx_skb = NULL; } dch->tx_idx = 0; if (dch->rx_skb) { - dev_kfree_skb(dch->rx_skb); + __skb_queue_tail(&free_queue, dch->rx_skb); dch->rx_skb = NULL; } test_and_clear_bit(FLG_TX_BUSY, &dch->Flags); if (test_and_clear_bit(FLG_BUSY_TIMER, &dch->Flags)) del_timer(&dch->timer); spin_unlock_irqrestore(&hc->lock, flags); + __skb_queue_purge(&free_queue); break; case HW_POWERUP_REQ: spin_lock_irqsave(&hc->lock, flags); @@ -3384,6 +3387,9 @@ handle_dmsg(struct mISDNchannel *ch, struct sk_buff *skb) case PH_DEACTIVATE_REQ: test_and_clear_bit(FLG_L2_ACTIVATED, &dch->Flags); if (dch->dev.D.protocol != ISDN_P_TE_S0) { + struct sk_buff_head free_queue; + + __skb_queue_head_init(&free_queue); spin_lock_irqsave(&hc->lock, flags); if (debug & DEBUG_HFCMULTI_MSG) printk(KERN_DEBUG @@ -3405,14 +3411,14 @@ handle_dmsg(struct mISDNchannel *ch, struct sk_buff *skb) /* deactivate */ dch->state = 1; } - skb_queue_purge(&dch->squeue); + skb_queue_splice_init(&dch->squeue, &free_queue); if (dch->tx_skb) { - dev_kfree_skb(dch->tx_skb); + __skb_queue_tail(&free_queue, dch->tx_skb); dch->tx_skb = NULL; } dch->tx_idx = 0; if (dch->rx_skb) { - dev_kfree_skb(dch->rx_skb); + __skb_queue_tail(&free_queue, dch->rx_skb); dch->rx_skb = NULL; } test_and_clear_bit(FLG_TX_BUSY, &dch->Flags); @@ -3424,6 +3430,7 @@ handle_dmsg(struct mISDNchannel *ch, struct sk_buff *skb) #endif ret = 0; spin_unlock_irqrestore(&hc->lock, flags); + __skb_queue_purge(&free_queue); } else ret = l1_event(dch->l1, hh->prim); break;
It is not allowed to call kfree_skb() or consume_skb() from hardware interrupt context or with hardware interrupts being disabled. skb_queue_purge() is called under spin_lock_irqsave() in handle_dmsg() and hfcm_l1callback(), kfree_skb() is called in them, to fix this, use skb_queue_splice_init() to move the dch->squeue to a free queue, also enqueue the tx_skb and rx_skb, at last calling __skb_queue_purge() to free the SKBs afer unlock. Fixes: af69fb3a8ffa ("Add mISDN HFC multiport driver") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- drivers/isdn/hardware/mISDN/hfcmulti.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)