* [PATCH net] ethernet: et131x: Add missing check after DMA map
@ 2025-07-07 9:09 Thomas Fourier
2025-07-07 20:01 ` Simon Horman
0 siblings, 1 reply; 3+ messages in thread
From: Thomas Fourier @ 2025-07-07 9:09 UTC (permalink / raw)
Cc: Thomas Fourier, Mark Einon, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Thomas Gleixner,
Ingo Molnar, netdev, linux-kernel
The DMA map functions can fail and should be tested for errors.
If the mapping fails, unmap and return an error.
Fixes: 38df6492eb51 ("et131x: Add PCIe gigabit ethernet driver et131x to drivers/net")
Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
---
drivers/net/ethernet/agere/et131x.c | 37 +++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/net/ethernet/agere/et131x.c b/drivers/net/ethernet/agere/et131x.c
index 678eddb36172..b32e2a5af779 100644
--- a/drivers/net/ethernet/agere/et131x.c
+++ b/drivers/net/ethernet/agere/et131x.c
@@ -2459,6 +2459,10 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
skb->data,
skb_headlen(skb),
DMA_TO_DEVICE);
+ if (dma_mapping_error(&adapter->pdev->dev,
+ dma_addr))
+ return -ENOMEM;
+
desc[frag].addr_lo = lower_32_bits(dma_addr);
desc[frag].addr_hi = upper_32_bits(dma_addr);
frag++;
@@ -2468,6 +2472,10 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
skb->data,
skb_headlen(skb) / 2,
DMA_TO_DEVICE);
+ if (dma_mapping_error(&adapter->pdev->dev,
+ dma_addr))
+ return -ENOMEM;
+
desc[frag].addr_lo = lower_32_bits(dma_addr);
desc[frag].addr_hi = upper_32_bits(dma_addr);
frag++;
@@ -2478,6 +2486,10 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
skb_headlen(skb) / 2,
skb_headlen(skb) / 2,
DMA_TO_DEVICE);
+ if (dma_mapping_error(&adapter->pdev->dev,
+ dma_addr))
+ goto unmap_first_out;
+
desc[frag].addr_lo = lower_32_bits(dma_addr);
desc[frag].addr_hi = upper_32_bits(dma_addr);
frag++;
@@ -2489,6 +2501,9 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
0,
desc[frag].len_vlan,
DMA_TO_DEVICE);
+ if (dma_mapping_error(&adapter->pdev->dev, dma_addr))
+ goto unmap_out;
+
desc[frag].addr_lo = lower_32_bits(dma_addr);
desc[frag].addr_hi = upper_32_bits(dma_addr);
frag++;
@@ -2578,6 +2593,28 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
&adapter->regs->global.watchdog_timer);
}
return 0;
+
+unmap_out:
+ // Unmap everything from i-1 to 1
+ while (--i) {
+ frag--;
+ dma_addr = desc[frag].addr_lo;
+ dma_addr |= (u64)desc[frag].addr_hi << 32;
+ dma_unmap_page(&adapter->pdev->dev, dma_addr,
+ desc[frag].len_vlan, DMA_TO_DEVICE);
+ }
+
+unmap_first_out:
+ // unmap header
+ while (frag--) {
+ frag--;
+ dma_addr = desc[frag].addr_lo;
+ dma_addr |= (u64)desc[frag].addr_hi << 32;
+ dma_unmap_single(&adapter->pdev->dev, dma_addr,
+ desc[frag].len_vlan, DMA_TO_DEVICE);
+ }
+
+ return -ENOMEM;
}
static int send_packet(struct sk_buff *skb, struct et131x_adapter *adapter)
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net] ethernet: et131x: Add missing check after DMA map
2025-07-07 9:09 [PATCH net] ethernet: et131x: Add missing check after DMA map Thomas Fourier
@ 2025-07-07 20:01 ` Simon Horman
2025-07-08 7:13 ` Thomas Fourier
0 siblings, 1 reply; 3+ messages in thread
From: Simon Horman @ 2025-07-07 20:01 UTC (permalink / raw)
To: Thomas Fourier
Cc: Mark Einon, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Thomas Gleixner, Ingo Molnar, netdev,
linux-kernel
On Mon, Jul 07, 2025 at 11:09:49AM +0200, Thomas Fourier wrote:
> The DMA map functions can fail and should be tested for errors.
> If the mapping fails, unmap and return an error.
>
> Fixes: 38df6492eb51 ("et131x: Add PCIe gigabit ethernet driver et131x to drivers/net")
> Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
nits:
1) There are two spaces after "et131x:" in the subject.
One is enough.
2) I think you can drop "ethernet: " from the subject.
"et131x: " seems to be an appropriate prefix based on git history.
...
> @@ -2578,6 +2593,28 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
> &adapter->regs->global.watchdog_timer);
> }
> return 0;
> +
> +unmap_out:
> + // Unmap everything from i-1 to 1
> + while (--i) {
> + frag--;
> + dma_addr = desc[frag].addr_lo;
> + dma_addr |= (u64)desc[frag].addr_hi << 32;
> + dma_unmap_page(&adapter->pdev->dev, dma_addr,
> + desc[frag].len_vlan, DMA_TO_DEVICE);
> + }
I'm probably missing something obvious. But it seems to me that frag is
incremented iff a mapping is successful. So I think only the loop below is
needed.
> +
> +unmap_first_out:
> + // unmap header
> + while (frag--) {
> + frag--;
I don't think you want to decrement frag twice here.
> + dma_addr = desc[frag].addr_lo;
> + dma_addr |= (u64)desc[frag].addr_hi << 32;
> + dma_unmap_single(&adapter->pdev->dev, dma_addr,
> + desc[frag].len_vlan, DMA_TO_DEVICE);
> + }
> +
> + return -ENOMEM;
> }
>
> static int send_packet(struct sk_buff *skb, struct et131x_adapter *adapter)
--
pw-bot: changes-requested
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net] ethernet: et131x: Add missing check after DMA map
2025-07-07 20:01 ` Simon Horman
@ 2025-07-08 7:13 ` Thomas Fourier
0 siblings, 0 replies; 3+ messages in thread
From: Thomas Fourier @ 2025-07-08 7:13 UTC (permalink / raw)
To: Simon Horman
Cc: Mark Einon, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Thomas Gleixner, Ingo Molnar, netdev,
linux-kernel
On 07/07/2025 22:01, Simon Horman wrote:
> On Mon, Jul 07, 2025 at 11:09:49AM +0200, Thomas Fourier wrote:
>> The DMA map functions can fail and should be tested for errors.
>> If the mapping fails, unmap and return an error.
>>
>> Fixes: 38df6492eb51 ("et131x: Add PCIe gigabit ethernet driver et131x to drivers/net")
>> Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
> nits:
>
> 1) There are two spaces after "et131x:" in the subject.
> One is enough.
>
> 2) I think you can drop "ethernet: " from the subject.
> "et131x: " seems to be an appropriate prefix based on git history.
>
> ...
Ok, I'll make sure to fix that.
>
>> @@ -2578,6 +2593,28 @@ static int nic_send_packet(struct et131x_adapter *adapter, struct tcb *tcb)
>> &adapter->regs->global.watchdog_timer);
>> }
>> return 0;
>> +
>> +unmap_out:
>> + // Unmap everything from i-1 to 1
>> + while (--i) {
>> + frag--;
>> + dma_addr = desc[frag].addr_lo;
>> + dma_addr |= (u64)desc[frag].addr_hi << 32;
>> + dma_unmap_page(&adapter->pdev->dev, dma_addr,
>> + desc[frag].len_vlan, DMA_TO_DEVICE);
>> + }
> I'm probably missing something obvious. But it seems to me that frag is
> incremented iff a mapping is successful. So I think only the loop below is
> needed.
Yes, frag is incremented only after a successful mapping.
The first loop is to unmap the body of the packet (i >= 1) which is mapped
with skb_frag_dma_map() (and unmapped with dma_unmap_page). The
second is to unmap the header which is either one or two mappings of
map_single. Should I make the comments more explicit?
>
>> +
>> +unmap_first_out:
>> + // unmap header
>> + while (frag--) {
>> + frag--;
> I don't think you want to decrement frag twice here.
>
>> + dma_addr = desc[frag].addr_lo;
>> + dma_addr |= (u64)desc[frag].addr_hi << 32;
>> + dma_unmap_single(&adapter->pdev->dev, dma_addr,
>> + desc[frag].len_vlan, DMA_TO_DEVICE);
>> + }
>> +
>> + return -ENOMEM;
>> }
>>
>> static int send_packet(struct sk_buff *skb, struct et131x_adapter *adapter)
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-07-08 7:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-07 9:09 [PATCH net] ethernet: et131x: Add missing check after DMA map Thomas Fourier
2025-07-07 20:01 ` Simon Horman
2025-07-08 7:13 ` Thomas Fourier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.