Message ID | 20190108124518.21986-1-baijiaju1990@gmail.com |
---|---|
State | Rejected |
Delegated to: | David Miller |
Headers | show |
Series | net: nvidia: forcedeth: Fix two possible concurrency use-after-free bugs | expand |
在 2019/1/8 20:45, Jia-Ju Bai 写道: > In drivers/net/ethernet/nvidia/forcedeth.c, the functions > nv_start_xmit() and nv_start_xmit_optimized() can be concurrently > executed with nv_poll_controller(). > > nv_start_xmit > line 2321: prev_tx_ctx->skb = skb; > > nv_start_xmit_optimized > line 2479: prev_tx_ctx->skb = skb; > > nv_poll_controller > nv_do_nic_poll > line 4134: spin_lock(&np->lock); > nv_drain_rxtx > nv_drain_tx > nv_release_txskb > line 2004: dev_kfree_skb_any(tx_skb->skb); > > Thus, two possible concurrency use-after-free bugs may occur. > > To fix these possible bugs, Does this really occur? Can you reproduce this ? > the calls to spin_lock_irqsave() in > nv_start_xmit() and nv_start_xmit_optimized() are moved to the > front of "prev_tx_ctx->skb = skb;" > > Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> > --- > drivers/net/ethernet/nvidia/forcedeth.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c > index 1d9b0d44ddb6..48fa5a0bd2cb 100644 > --- a/drivers/net/ethernet/nvidia/forcedeth.c > +++ b/drivers/net/ethernet/nvidia/forcedeth.c > @@ -2317,6 +2317,8 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev) > /* set last fragment flag */ > prev_tx->flaglen |= cpu_to_le32(tx_flags_extra); > > + spin_lock_irqsave(&np->lock, flags); > + > /* save skb in this slot's context area */ > prev_tx_ctx->skb = skb; > > @@ -2326,8 +2328,6 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev) > tx_flags_extra = skb->ip_summed == CHECKSUM_PARTIAL ? > NV_TX2_CHECKSUM_L3 | NV_TX2_CHECKSUM_L4 : 0; > > - spin_lock_irqsave(&np->lock, flags); > - > /* set tx flags */ > start_tx->flaglen |= cpu_to_le32(tx_flags | tx_flags_extra); > > @@ -2475,6 +2475,8 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb, > /* set last fragment flag */ > prev_tx->flaglen |= cpu_to_le32(NV_TX2_LASTPACKET); > > + spin_lock_irqsave(&np->lock, flags); > + > /* save skb in this slot's context area */ > prev_tx_ctx->skb = skb; > > @@ -2491,8 +2493,6 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb, > else > start_tx->txvlan = 0; > > - spin_lock_irqsave(&np->lock, flags); > - > if (np->tx_limit) { > /* Limit the number of outstanding tx. Setup all fragments, but > * do not set the VALID bit on the first descriptor. Save a pointer
On 2019/1/8 20:54, Zhu Yanjun wrote: > > 在 2019/1/8 20:45, Jia-Ju Bai 写道: >> In drivers/net/ethernet/nvidia/forcedeth.c, the functions >> nv_start_xmit() and nv_start_xmit_optimized() can be concurrently >> executed with nv_poll_controller(). >> >> nv_start_xmit >> line 2321: prev_tx_ctx->skb = skb; >> >> nv_start_xmit_optimized >> line 2479: prev_tx_ctx->skb = skb; >> >> nv_poll_controller >> nv_do_nic_poll >> line 4134: spin_lock(&np->lock); >> nv_drain_rxtx >> nv_drain_tx >> nv_release_txskb >> line 2004: dev_kfree_skb_any(tx_skb->skb); >> >> Thus, two possible concurrency use-after-free bugs may occur. >> >> To fix these possible bugs, > > > Does this really occur? Can you reproduce this ? This bug is not found by the real execution. It is found by a static tool written by myself, and then I check it by manual code review. Best wishes, Jia-Ju Bai
On 2019/1/8 20:57, Jia-Ju Bai wrote: > > > On 2019/1/8 20:54, Zhu Yanjun wrote: >> >> 在 2019/1/8 20:45, Jia-Ju Bai 写道: >>> In drivers/net/ethernet/nvidia/forcedeth.c, the functions >>> nv_start_xmit() and nv_start_xmit_optimized() can be concurrently >>> executed with nv_poll_controller(). >>> >>> nv_start_xmit >>> line 2321: prev_tx_ctx->skb = skb; >>> >>> nv_start_xmit_optimized >>> line 2479: prev_tx_ctx->skb = skb; >>> >>> nv_poll_controller >>> nv_do_nic_poll >>> line 4134: spin_lock(&np->lock); >>> nv_drain_rxtx >>> nv_drain_tx >>> nv_release_txskb >>> line 2004: dev_kfree_skb_any(tx_skb->skb); >>> >>> Thus, two possible concurrency use-after-free bugs may occur. >>> >>> To fix these possible bugs, >> >> >> Does this really occur? Can you reproduce this ? > > This bug is not found by the real execution. > It is found by a static tool written by myself, and then I check it by > manual code review. Before "line 2004: dev_kfree_skb_any(tx_skb->skb); ", " nv_disable_irq(dev); nv_napi_disable(dev); netif_tx_lock_bh(dev); netif_addr_lock(dev); spin_lock(&np->lock); /* stop engines */ nv_stop_rxtx(dev); <---this stop rxtx nv_txrx_reset(dev); " In this case, does nv_start_xmit or nv_start_xmit_optimized still work well? Zhu Yanjun > > > Best wishes, > Jia-Ju Bai
On 2019/1/9 9:24, Yanjun Zhu wrote: > > On 2019/1/8 20:57, Jia-Ju Bai wrote: >> >> >> On 2019/1/8 20:54, Zhu Yanjun wrote: >>> >>> 在 2019/1/8 20:45, Jia-Ju Bai 写道: >>>> In drivers/net/ethernet/nvidia/forcedeth.c, the functions >>>> nv_start_xmit() and nv_start_xmit_optimized() can be concurrently >>>> executed with nv_poll_controller(). >>>> >>>> nv_start_xmit >>>> line 2321: prev_tx_ctx->skb = skb; >>>> >>>> nv_start_xmit_optimized >>>> line 2479: prev_tx_ctx->skb = skb; >>>> >>>> nv_poll_controller >>>> nv_do_nic_poll >>>> line 4134: spin_lock(&np->lock); >>>> nv_drain_rxtx >>>> nv_drain_tx >>>> nv_release_txskb >>>> line 2004: dev_kfree_skb_any(tx_skb->skb); >>>> >>>> Thus, two possible concurrency use-after-free bugs may occur. >>>> >>>> To fix these possible bugs, >>> >>> >>> Does this really occur? Can you reproduce this ? >> >> This bug is not found by the real execution. >> It is found by a static tool written by myself, and then I check it >> by manual code review. > > Before "line 2004: dev_kfree_skb_any(tx_skb->skb); ", > > " > > nv_disable_irq(dev); > nv_napi_disable(dev); > netif_tx_lock_bh(dev); > netif_addr_lock(dev); > spin_lock(&np->lock); > /* stop engines */ > nv_stop_rxtx(dev); <---this stop rxtx > nv_txrx_reset(dev); > " > > In this case, does nv_start_xmit or nv_start_xmit_optimized still work > well? > nv_stop_rxtx() calls nv_stop_tx(dev). static void nv_stop_tx(struct net_device *dev) { struct fe_priv *np = netdev_priv(dev); u8 __iomem *base = get_hwbase(dev); u32 tx_ctrl = readl(base + NvRegTransmitterControl); if (!np->mac_in_use) tx_ctrl &= ~NVREG_XMITCTL_START; else tx_ctrl |= NVREG_XMITCTL_TX_PATH_EN; writel(tx_ctrl, base + NvRegTransmitterControl); if (reg_delay(dev, NvRegTransmitterStatus, NVREG_XMITSTAT_BUSY, 0, NV_TXSTOP_DELAY1, NV_TXSTOP_DELAY1MAX)) netdev_info(dev, "%s: TransmitterStatus remained busy\n", __func__); udelay(NV_TXSTOP_DELAY2); if (!np->mac_in_use) writel(readl(base + NvRegTransmitPoll) & NVREG_TRANSMITPOLL_MAC_ADDR_REV, base + NvRegTransmitPoll); } nv_stop_tx() seems to only write registers to stop transmitting for hardware. But it does not wait until nv_start_xmit() and nv_start_xmit_optimized() finish execution. Maybe netif_stop_queue() should be used here to stop transmitting for network layer, but this function does not seem to wait, either. Do you know any function that can wait until ".ndo_start_xmit" finish execution? Best wishes, Jia-Ju Bai
On 2019/1/9 10:03, Jia-Ju Bai wrote: > > > On 2019/1/9 9:24, Yanjun Zhu wrote: >> >> On 2019/1/8 20:57, Jia-Ju Bai wrote: >>> >>> >>> On 2019/1/8 20:54, Zhu Yanjun wrote: >>>> >>>> 在 2019/1/8 20:45, Jia-Ju Bai 写道: >>>>> In drivers/net/ethernet/nvidia/forcedeth.c, the functions >>>>> nv_start_xmit() and nv_start_xmit_optimized() can be concurrently >>>>> executed with nv_poll_controller(). >>>>> >>>>> nv_start_xmit >>>>> line 2321: prev_tx_ctx->skb = skb; >>>>> >>>>> nv_start_xmit_optimized >>>>> line 2479: prev_tx_ctx->skb = skb; >>>>> >>>>> nv_poll_controller >>>>> nv_do_nic_poll >>>>> line 4134: spin_lock(&np->lock); >>>>> nv_drain_rxtx >>>>> nv_drain_tx >>>>> nv_release_txskb >>>>> line 2004: dev_kfree_skb_any(tx_skb->skb); >>>>> >>>>> Thus, two possible concurrency use-after-free bugs may occur. >>>>> >>>>> To fix these possible bugs, >>>> >>>> >>>> Does this really occur? Can you reproduce this ? >>> >>> This bug is not found by the real execution. >>> It is found by a static tool written by myself, and then I check it >>> by manual code review. >> >> Before "line 2004: dev_kfree_skb_any(tx_skb->skb); ", >> >> " >> >> nv_disable_irq(dev); >> nv_napi_disable(dev); >> netif_tx_lock_bh(dev); >> netif_addr_lock(dev); >> spin_lock(&np->lock); >> /* stop engines */ >> nv_stop_rxtx(dev); <---this stop rxtx >> nv_txrx_reset(dev); >> " >> >> In this case, does nv_start_xmit or nv_start_xmit_optimized still >> work well? >> > > nv_stop_rxtx() calls nv_stop_tx(dev). > > static void nv_stop_tx(struct net_device *dev) > { > struct fe_priv *np = netdev_priv(dev); > u8 __iomem *base = get_hwbase(dev); > u32 tx_ctrl = readl(base + NvRegTransmitterControl); > > if (!np->mac_in_use) > tx_ctrl &= ~NVREG_XMITCTL_START; > else > tx_ctrl |= NVREG_XMITCTL_TX_PATH_EN; > writel(tx_ctrl, base + NvRegTransmitterControl); > if (reg_delay(dev, NvRegTransmitterStatus, NVREG_XMITSTAT_BUSY, 0, > NV_TXSTOP_DELAY1, NV_TXSTOP_DELAY1MAX)) > netdev_info(dev, "%s: TransmitterStatus remained busy\n", > __func__); > > udelay(NV_TXSTOP_DELAY2); > if (!np->mac_in_use) > writel(readl(base + NvRegTransmitPoll) & > NVREG_TRANSMITPOLL_MAC_ADDR_REV, > base + NvRegTransmitPoll); > } > > nv_stop_tx() seems to only write registers to stop transmitting for > hardware. > But it does not wait until nv_start_xmit() and > nv_start_xmit_optimized() finish execution. There are 3 modes in forcedeth NIC. In throughput mode (0), every tx & rx packet will generate an interrupt. In CPU mode (1), interrupts are controlled by a timer. In dynamic mode (2), the mode toggles between throughput and CPU mode based on network load. From the source code, "np->recover_error = 1;" is related with CPU mode. nv_start_xmit or nv_start_xmit_optimized seems related with ghroughput mode. In static void nv_do_nic_poll(struct timer_list *t), when if (np->recover_error), line 2004: dev_kfree_skb_any(tx_skb->skb); will run. When "np->recover_error=1", do you think nv_start_xmit or nv_start_xmit_optimized will be called? > Maybe netif_stop_queue() should be used here to stop transmitting for > network layer, but this function does not seem to wait, either. > Do you know any function that can wait until ".ndo_start_xmit" finish > execution? > > > Best wishes, > Jia-Ju Bai > >
On 2019/1/9 10:35, Yanjun Zhu wrote: > > On 2019/1/9 10:03, Jia-Ju Bai wrote: >> >> >> On 2019/1/9 9:24, Yanjun Zhu wrote: >>> >>> On 2019/1/8 20:57, Jia-Ju Bai wrote: >>>> >>>> >>>> On 2019/1/8 20:54, Zhu Yanjun wrote: >>>>> >>>>> 在 2019/1/8 20:45, Jia-Ju Bai 写道: >>>>>> In drivers/net/ethernet/nvidia/forcedeth.c, the functions >>>>>> nv_start_xmit() and nv_start_xmit_optimized() can be concurrently >>>>>> executed with nv_poll_controller(). >>>>>> >>>>>> nv_start_xmit >>>>>> line 2321: prev_tx_ctx->skb = skb; >>>>>> >>>>>> nv_start_xmit_optimized >>>>>> line 2479: prev_tx_ctx->skb = skb; >>>>>> >>>>>> nv_poll_controller >>>>>> nv_do_nic_poll >>>>>> line 4134: spin_lock(&np->lock); >>>>>> nv_drain_rxtx >>>>>> nv_drain_tx >>>>>> nv_release_txskb >>>>>> line 2004: dev_kfree_skb_any(tx_skb->skb); >>>>>> >>>>>> Thus, two possible concurrency use-after-free bugs may occur. >>>>>> >>>>>> To fix these possible bugs, >>>>> >>>>> >>>>> Does this really occur? Can you reproduce this ? >>>> >>>> This bug is not found by the real execution. >>>> It is found by a static tool written by myself, and then I check it >>>> by manual code review. >>> >>> Before "line 2004: dev_kfree_skb_any(tx_skb->skb); ", >>> >>> " >>> >>> nv_disable_irq(dev); >>> nv_napi_disable(dev); >>> netif_tx_lock_bh(dev); >>> netif_addr_lock(dev); >>> spin_lock(&np->lock); >>> /* stop engines */ >>> nv_stop_rxtx(dev); <---this stop rxtx >>> nv_txrx_reset(dev); >>> " >>> >>> In this case, does nv_start_xmit or nv_start_xmit_optimized still >>> work well? >>> >> >> nv_stop_rxtx() calls nv_stop_tx(dev). >> >> static void nv_stop_tx(struct net_device *dev) >> { >> struct fe_priv *np = netdev_priv(dev); >> u8 __iomem *base = get_hwbase(dev); >> u32 tx_ctrl = readl(base + NvRegTransmitterControl); >> >> if (!np->mac_in_use) >> tx_ctrl &= ~NVREG_XMITCTL_START; >> else >> tx_ctrl |= NVREG_XMITCTL_TX_PATH_EN; >> writel(tx_ctrl, base + NvRegTransmitterControl); >> if (reg_delay(dev, NvRegTransmitterStatus, NVREG_XMITSTAT_BUSY, 0, >> NV_TXSTOP_DELAY1, NV_TXSTOP_DELAY1MAX)) >> netdev_info(dev, "%s: TransmitterStatus remained busy\n", >> __func__); >> >> udelay(NV_TXSTOP_DELAY2); >> if (!np->mac_in_use) >> writel(readl(base + NvRegTransmitPoll) & >> NVREG_TRANSMITPOLL_MAC_ADDR_REV, >> base + NvRegTransmitPoll); >> } >> >> nv_stop_tx() seems to only write registers to stop transmitting for >> hardware. >> But it does not wait until nv_start_xmit() and >> nv_start_xmit_optimized() finish execution. > There are 3 modes in forcedeth NIC. > In throughput mode (0), every tx & rx packet will generate an interrupt. > In CPU mode (1), interrupts are controlled by a timer. > In dynamic mode (2), the mode toggles between throughput and CPU mode > based on network load. > > From the source code, > > "np->recover_error = 1;" is related with CPU mode. > > nv_start_xmit or nv_start_xmit_optimized seems related with ghroughput > mode. > > In static void nv_do_nic_poll(struct timer_list *t), > when if (np->recover_error), line 2004: > dev_kfree_skb_any(tx_skb->skb); will run. > > When "np->recover_error=1", do you think nv_start_xmit or > nv_start_xmit_optimized will be called? Sorry, I do not know about these modes... But I still think nv_start_xmit() or nv_start_xmit_optimized() can be called here, in no matter which mode :) Best wishes, Jia-Ju Bai
On 2019/1/9 11:20, Jia-Ju Bai wrote: > > > On 2019/1/9 10:35, Yanjun Zhu wrote: >> >> On 2019/1/9 10:03, Jia-Ju Bai wrote: >>> >>> >>> On 2019/1/9 9:24, Yanjun Zhu wrote: >>>> >>>> On 2019/1/8 20:57, Jia-Ju Bai wrote: >>>>> >>>>> >>>>> On 2019/1/8 20:54, Zhu Yanjun wrote: >>>>>> >>>>>> 在 2019/1/8 20:45, Jia-Ju Bai 写道: >>>>>>> In drivers/net/ethernet/nvidia/forcedeth.c, the functions >>>>>>> nv_start_xmit() and nv_start_xmit_optimized() can be concurrently >>>>>>> executed with nv_poll_controller(). >>>>>>> >>>>>>> nv_start_xmit >>>>>>> line 2321: prev_tx_ctx->skb = skb; >>>>>>> >>>>>>> nv_start_xmit_optimized >>>>>>> line 2479: prev_tx_ctx->skb = skb; >>>>>>> >>>>>>> nv_poll_controller >>>>>>> nv_do_nic_poll >>>>>>> line 4134: spin_lock(&np->lock); >>>>>>> nv_drain_rxtx >>>>>>> nv_drain_tx >>>>>>> nv_release_txskb >>>>>>> line 2004: dev_kfree_skb_any(tx_skb->skb); >>>>>>> >>>>>>> Thus, two possible concurrency use-after-free bugs may occur. >>>>>>> >>>>>>> To fix these possible bugs, >>>>>> >>>>>> >>>>>> Does this really occur? Can you reproduce this ? >>>>> >>>>> This bug is not found by the real execution. >>>>> It is found by a static tool written by myself, and then I check >>>>> it by manual code review. >>>> >>>> Before "line 2004: dev_kfree_skb_any(tx_skb->skb); ", >>>> >>>> " >>>> >>>> nv_disable_irq(dev); >>>> nv_napi_disable(dev); >>>> netif_tx_lock_bh(dev); >>>> netif_addr_lock(dev); >>>> spin_lock(&np->lock); >>>> /* stop engines */ >>>> nv_stop_rxtx(dev); <---this stop rxtx >>>> nv_txrx_reset(dev); >>>> " >>>> >>>> In this case, does nv_start_xmit or nv_start_xmit_optimized still >>>> work well? >>>> >>> >>> nv_stop_rxtx() calls nv_stop_tx(dev). >>> >>> static void nv_stop_tx(struct net_device *dev) >>> { >>> struct fe_priv *np = netdev_priv(dev); >>> u8 __iomem *base = get_hwbase(dev); >>> u32 tx_ctrl = readl(base + NvRegTransmitterControl); >>> >>> if (!np->mac_in_use) >>> tx_ctrl &= ~NVREG_XMITCTL_START; >>> else >>> tx_ctrl |= NVREG_XMITCTL_TX_PATH_EN; >>> writel(tx_ctrl, base + NvRegTransmitterControl); >>> if (reg_delay(dev, NvRegTransmitterStatus, NVREG_XMITSTAT_BUSY, 0, >>> NV_TXSTOP_DELAY1, NV_TXSTOP_DELAY1MAX)) >>> netdev_info(dev, "%s: TransmitterStatus remained busy\n", >>> __func__); >>> >>> udelay(NV_TXSTOP_DELAY2); >>> if (!np->mac_in_use) >>> writel(readl(base + NvRegTransmitPoll) & >>> NVREG_TRANSMITPOLL_MAC_ADDR_REV, >>> base + NvRegTransmitPoll); >>> } >>> >>> nv_stop_tx() seems to only write registers to stop transmitting for >>> hardware. >>> But it does not wait until nv_start_xmit() and >>> nv_start_xmit_optimized() finish execution. >> There are 3 modes in forcedeth NIC. >> In throughput mode (0), every tx & rx packet will generate an interrupt. >> In CPU mode (1), interrupts are controlled by a timer. >> In dynamic mode (2), the mode toggles between throughput and CPU mode >> based on network load. >> >> From the source code, >> >> "np->recover_error = 1;" is related with CPU mode. >> >> nv_start_xmit or nv_start_xmit_optimized seems related with >> ghroughput mode. >> >> In static void nv_do_nic_poll(struct timer_list *t), >> when if (np->recover_error), line 2004: >> dev_kfree_skb_any(tx_skb->skb); will run. >> >> When "np->recover_error=1", do you think nv_start_xmit or >> nv_start_xmit_optimized will be called? > > Sorry, I do not know about these modes... > But I still think nv_start_xmit() or nv_start_xmit_optimized() can be > called here, in no matter which mode :) :-P If you have forcedeth NIC, you can make tests with it.:-) > > > Best wishes, > Jia-Ju Bai >
On 2019/1/9 11:24, Yanjun Zhu wrote: > > If you have forcedeth NIC, you can make tests with it.:-) Ah, I would like to, but I do not have the hardware... Best wishes, Jia-Ju Bai
From: Jia-Ju Bai <baijiaju1990@gmail.com> Date: Tue, 8 Jan 2019 20:45:18 +0800 > In drivers/net/ethernet/nvidia/forcedeth.c, the functions > nv_start_xmit() and nv_start_xmit_optimized() can be concurrently > executed with nv_poll_controller(). > > nv_start_xmit > line 2321: prev_tx_ctx->skb = skb; > > nv_start_xmit_optimized > line 2479: prev_tx_ctx->skb = skb; > > nv_poll_controller > nv_do_nic_poll > line 4134: spin_lock(&np->lock); > nv_drain_rxtx > nv_drain_tx > nv_release_txskb > line 2004: dev_kfree_skb_any(tx_skb->skb); > > Thus, two possible concurrency use-after-free bugs may occur. I do not think so, the netif_tx_lock_bh() done will prevent the parallel execution.
diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c index 1d9b0d44ddb6..48fa5a0bd2cb 100644 --- a/drivers/net/ethernet/nvidia/forcedeth.c +++ b/drivers/net/ethernet/nvidia/forcedeth.c @@ -2317,6 +2317,8 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev) /* set last fragment flag */ prev_tx->flaglen |= cpu_to_le32(tx_flags_extra); + spin_lock_irqsave(&np->lock, flags); + /* save skb in this slot's context area */ prev_tx_ctx->skb = skb; @@ -2326,8 +2328,6 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev) tx_flags_extra = skb->ip_summed == CHECKSUM_PARTIAL ? NV_TX2_CHECKSUM_L3 | NV_TX2_CHECKSUM_L4 : 0; - spin_lock_irqsave(&np->lock, flags); - /* set tx flags */ start_tx->flaglen |= cpu_to_le32(tx_flags | tx_flags_extra); @@ -2475,6 +2475,8 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb, /* set last fragment flag */ prev_tx->flaglen |= cpu_to_le32(NV_TX2_LASTPACKET); + spin_lock_irqsave(&np->lock, flags); + /* save skb in this slot's context area */ prev_tx_ctx->skb = skb; @@ -2491,8 +2493,6 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb, else start_tx->txvlan = 0; - spin_lock_irqsave(&np->lock, flags); - if (np->tx_limit) { /* Limit the number of outstanding tx. Setup all fragments, but * do not set the VALID bit on the first descriptor. Save a pointer
In drivers/net/ethernet/nvidia/forcedeth.c, the functions nv_start_xmit() and nv_start_xmit_optimized() can be concurrently executed with nv_poll_controller(). nv_start_xmit line 2321: prev_tx_ctx->skb = skb; nv_start_xmit_optimized line 2479: prev_tx_ctx->skb = skb; nv_poll_controller nv_do_nic_poll line 4134: spin_lock(&np->lock); nv_drain_rxtx nv_drain_tx nv_release_txskb line 2004: dev_kfree_skb_any(tx_skb->skb); Thus, two possible concurrency use-after-free bugs may occur. To fix these possible bugs, the calls to spin_lock_irqsave() in nv_start_xmit() and nv_start_xmit_optimized() are moved to the front of "prev_tx_ctx->skb = skb;" Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> --- drivers/net/ethernet/nvidia/forcedeth.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)