| Message ID | 20241202134904.3882317-2-nikita.yoush@cogentembedded.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: renesas: rswitch: several fixes | expand |
On 12/2/2024 5:49 AM, Nikita Yushchenko wrote: > When sending frame split into multiple descriptors, hardware processes > descriptors one by one, including writing back DT values. The first > descriptor could be already marked as completed when processing of > next descriptors for the same frame is still in progress. > > Although only the last descriptor is configured to generate interrupt, > completion of the first descriptor could be noticed by the driver when > handling interrupt for the previous frame. > > Currently, driver stores skb in the entry that corresponds to the first > descriptor. This results into skb could be unmapped and freed when > hardware did not complete the send yet. This opens a window for > corrupting the data being sent. > > Fix this by saving skb in the entry that corresponds to the last > descriptor used to send the frame. > > Fixes: d2c96b9d5f83 ("net: rswitch: Add jumbo frames handling for TX") > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> > --- > drivers/net/ethernet/renesas/rswitch.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/renesas/rswitch.c b/drivers/net/ethernet/renesas/rswitch.c > index b80aa27a7214..32b32aa7e01f 100644 > --- a/drivers/net/ethernet/renesas/rswitch.c > +++ b/drivers/net/ethernet/renesas/rswitch.c > @@ -1681,8 +1681,9 @@ static netdev_tx_t rswitch_start_xmit(struct sk_buff *skb, struct net_device *nd > if (dma_mapping_error(ndev->dev.parent, dma_addr_orig)) > goto err_kfree; > > - gq->skbs[gq->cur] = skb; > - gq->unmap_addrs[gq->cur] = dma_addr_orig; > + /* Stored the skb at the last descriptor to avoid skb free before hardware completes send */ > + gq->skbs[(gq->cur + nr_desc - 1) % gq->ring_size] = skb; > + gq->unmap_addrs[(gq->cur + nr_desc - 1) % gq->ring_size] = dma_addr_orig; > nr_desc is non-zero, so if nr_desc was 1, then this would point to gq->cur + 0 mod ring_size, i.e. gq->cur. I might have possibly computed that separately as a local variable since this expression is repeated twice, but I don't think thats going to do too much for readability either way. Ok Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> > /* DT_FSTART should be set at last. So, this is reverse order. */ > for (i = nr_desc; i-- > 0; ) {
diff --git a/drivers/net/ethernet/renesas/rswitch.c b/drivers/net/ethernet/renesas/rswitch.c index b80aa27a7214..32b32aa7e01f 100644 --- a/drivers/net/ethernet/renesas/rswitch.c +++ b/drivers/net/ethernet/renesas/rswitch.c @@ -1681,8 +1681,9 @@ static netdev_tx_t rswitch_start_xmit(struct sk_buff *skb, struct net_device *nd if (dma_mapping_error(ndev->dev.parent, dma_addr_orig)) goto err_kfree; - gq->skbs[gq->cur] = skb; - gq->unmap_addrs[gq->cur] = dma_addr_orig; + /* Stored the skb at the last descriptor to avoid skb free before hardware completes send */ + gq->skbs[(gq->cur + nr_desc - 1) % gq->ring_size] = skb; + gq->unmap_addrs[(gq->cur + nr_desc - 1) % gq->ring_size] = dma_addr_orig; /* DT_FSTART should be set at last. So, this is reverse order. */ for (i = nr_desc; i-- > 0; ) {
When sending frame split into multiple descriptors, hardware processes descriptors one by one, including writing back DT values. The first descriptor could be already marked as completed when processing of next descriptors for the same frame is still in progress. Although only the last descriptor is configured to generate interrupt, completion of the first descriptor could be noticed by the driver when handling interrupt for the previous frame. Currently, driver stores skb in the entry that corresponds to the first descriptor. This results into skb could be unmapped and freed when hardware did not complete the send yet. This opens a window for corrupting the data being sent. Fix this by saving skb in the entry that corresponds to the last descriptor used to send the frame. Fixes: d2c96b9d5f83 ("net: rswitch: Add jumbo frames handling for TX") Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> --- drivers/net/ethernet/renesas/rswitch.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)