drivers/net/ethernet/atheros/atlx/atl1.c | 38 ++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-)
According to Shuah Khan[1], all `dma_map()` functions should be tested
before using the pointer. This patch checks for errors in `dma_map()`
calls and in case of failure, unmaps the previously dma_mapped regions
and returns an error.
[1] https://events.static.linuxfound.org/sites/events/files/slides/Shuah_Khan_dma_map_error.pdf
Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
---
drivers/net/ethernet/atheros/atlx/atl1.c | 38 ++++++++++++++++++++++--
1 file changed, 36 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index cfdb546a09e7..99d7a37b4ddf 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -1869,6 +1869,13 @@ static u16 atl1_alloc_rx_buffers(struct atl1_adapter *adapter)
buffer_info->dma = dma_map_page(&pdev->dev, page, offset,
adapter->rx_buffer_len,
DMA_FROM_DEVICE);
+ if (dma_mapping_error(&pdev->dev, buffer_info->dma)) {
+ buffer_info->alloced = 0;
+ buffer_info->skb = NULL;
+ kfree_skb(skb);
+ adapter->soft_stats.rx_dropped++;
+ break;
+ }
rfd_desc->buffer_addr = cpu_to_le64(buffer_info->dma);
rfd_desc->buf_len = cpu_to_le16(adapter->rx_buffer_len);
rfd_desc->coalese = 0;
@@ -2183,7 +2190,7 @@ static int atl1_tx_csum(struct atl1_adapter *adapter, struct sk_buff *skb,
return 0;
}
-static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
+static int atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
struct tx_packet_desc *ptpd)
{
struct atl1_tpd_ring *tpd_ring = &adapter->tpd_ring;
@@ -2195,12 +2202,14 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
unsigned int f;
int retval;
u16 next_to_use;
+ u16 first_mapped;
u16 data_len;
u8 hdr_len;
buf_len -= skb->data_len;
nr_frags = skb_shinfo(skb)->nr_frags;
next_to_use = atomic_read(&tpd_ring->next_to_use);
+ first_mapped = next_to_use;
buffer_info = &tpd_ring->buffer_info[next_to_use];
BUG_ON(buffer_info->skb);
/* put skb in last TPD */
@@ -2216,6 +2225,8 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
buffer_info->dma = dma_map_page(&adapter->pdev->dev, page,
offset, hdr_len,
DMA_TO_DEVICE);
+ if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma))
+ goto dma_err;
if (++next_to_use == tpd_ring->count)
next_to_use = 0;
@@ -2242,6 +2253,8 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
page, offset,
buffer_info->length,
DMA_TO_DEVICE);
+ if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma))
+ goto dma_err;
if (++next_to_use == tpd_ring->count)
next_to_use = 0;
}
@@ -2254,6 +2267,8 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
buffer_info->dma = dma_map_page(&adapter->pdev->dev, page,
offset, buf_len,
DMA_TO_DEVICE);
+ if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma))
+ goto dma_err;
if (++next_to_use == tpd_ring->count)
next_to_use = 0;
}
@@ -2277,6 +2292,8 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
buffer_info->dma = skb_frag_dma_map(&adapter->pdev->dev,
frag, i * ATL1_MAX_TX_BUF_LEN,
buffer_info->length, DMA_TO_DEVICE);
+ if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma))
+ goto dma_err;
if (++next_to_use == tpd_ring->count)
next_to_use = 0;
@@ -2285,6 +2302,22 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
/* last tpd's buffer-info */
buffer_info->skb = skb;
+
+ return 0;
+
+ dma_err:
+ while (first_mapped != next_to_use) {
+ buffer_info = &tpd_ring->buffer_info[first_mapped];
+ dma_unmap_page(&adapter->pdev->dev,
+ buffer_info->dma,
+ buffer_info->length,
+ DMA_TO_DEVICE);
+ buffer_info->dma = NULL;
+
+ if (++first_mapped == tdp_ring->count)
+ first_mapped = 0;
+ }
+ return -ENOMEM;
}
static void atl1_tx_queue(struct atl1_adapter *adapter, u16 count,
@@ -2419,7 +2452,8 @@ static netdev_tx_t atl1_xmit_frame(struct sk_buff *skb,
}
}
- atl1_tx_map(adapter, skb, ptpd);
+ if (atl1_tx_map(adapter, skb, ptpd))
+ return NETDEV_TX_BUSY;
atl1_tx_queue(adapter, count, ptpd);
atl1_update_mailbox(adapter);
return NETDEV_TX_OK;
--
2.43.0Hi Thomas,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.16-rc1 next-20250612]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Thomas-Fourier/drivers-ethernet-atheros-atl1-test-DMA-mapping-for-error-code/20250612-231733
base: linus/master
patch link: https://lore.kernel.org/r/20250612150542.85239-2-fourier.thomas%40gmail.com
patch subject: [PATCH] (drivers/ethernet/atheros/atl1) test DMA mapping for error code
config: x86_64-rhel-9.4 (https://download.01.org/0day-ci/archive/20250613/202506131458.MnU50xNO-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250613/202506131458.MnU50xNO-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506131458.MnU50xNO-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
drivers/net/ethernet/atheros/atlx/atl1.c: In function 'atl1_tx_map':
>> drivers/net/ethernet/atheros/atlx/atl1.c:2315:34: warning: assignment to 'dma_addr_t' {aka 'long long unsigned int'} from 'void *' makes integer from pointer without a cast [-Wint-conversion]
2315 | buffer_info->dma = NULL;
| ^
>> drivers/net/ethernet/atheros/atlx/atl1.c:2317:39: error: 'tdp_ring' undeclared (first use in this function); did you mean 'tpd_ring'?
2317 | if (++first_mapped == tdp_ring->count)
| ^~~~~~~~
| tpd_ring
drivers/net/ethernet/atheros/atlx/atl1.c:2317:39: note: each undeclared identifier is reported only once for each function it appears in
vim +2317 drivers/net/ethernet/atheros/atlx/atl1.c
2192
2193 static int atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
2194 struct tx_packet_desc *ptpd)
2195 {
2196 struct atl1_tpd_ring *tpd_ring = &adapter->tpd_ring;
2197 struct atl1_buffer *buffer_info;
2198 u16 buf_len = skb->len;
2199 struct page *page;
2200 unsigned long offset;
2201 unsigned int nr_frags;
2202 unsigned int f;
2203 int retval;
2204 u16 next_to_use;
2205 u16 first_mapped;
2206 u16 data_len;
2207 u8 hdr_len;
2208
2209 buf_len -= skb->data_len;
2210 nr_frags = skb_shinfo(skb)->nr_frags;
2211 next_to_use = atomic_read(&tpd_ring->next_to_use);
2212 first_mapped = next_to_use;
2213 buffer_info = &tpd_ring->buffer_info[next_to_use];
2214 BUG_ON(buffer_info->skb);
2215 /* put skb in last TPD */
2216 buffer_info->skb = NULL;
2217
2218 retval = (ptpd->word3 >> TPD_SEGMENT_EN_SHIFT) & TPD_SEGMENT_EN_MASK;
2219 if (retval) {
2220 /* TSO */
2221 hdr_len = skb_tcp_all_headers(skb);
2222 buffer_info->length = hdr_len;
2223 page = virt_to_page(skb->data);
2224 offset = offset_in_page(skb->data);
2225 buffer_info->dma = dma_map_page(&adapter->pdev->dev, page,
2226 offset, hdr_len,
2227 DMA_TO_DEVICE);
2228 if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma))
2229 goto dma_err;
2230
2231 if (++next_to_use == tpd_ring->count)
2232 next_to_use = 0;
2233
2234 if (buf_len > hdr_len) {
2235 int i, nseg;
2236
2237 data_len = buf_len - hdr_len;
2238 nseg = (data_len + ATL1_MAX_TX_BUF_LEN - 1) /
2239 ATL1_MAX_TX_BUF_LEN;
2240 for (i = 0; i < nseg; i++) {
2241 buffer_info =
2242 &tpd_ring->buffer_info[next_to_use];
2243 buffer_info->skb = NULL;
2244 buffer_info->length =
2245 (ATL1_MAX_TX_BUF_LEN >=
2246 data_len) ? ATL1_MAX_TX_BUF_LEN : data_len;
2247 data_len -= buffer_info->length;
2248 page = virt_to_page(skb->data +
2249 (hdr_len + i * ATL1_MAX_TX_BUF_LEN));
2250 offset = offset_in_page(skb->data +
2251 (hdr_len + i * ATL1_MAX_TX_BUF_LEN));
2252 buffer_info->dma = dma_map_page(&adapter->pdev->dev,
2253 page, offset,
2254 buffer_info->length,
2255 DMA_TO_DEVICE);
2256 if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma))
2257 goto dma_err;
2258 if (++next_to_use == tpd_ring->count)
2259 next_to_use = 0;
2260 }
2261 }
2262 } else {
2263 /* not TSO */
2264 buffer_info->length = buf_len;
2265 page = virt_to_page(skb->data);
2266 offset = offset_in_page(skb->data);
2267 buffer_info->dma = dma_map_page(&adapter->pdev->dev, page,
2268 offset, buf_len,
2269 DMA_TO_DEVICE);
2270 if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma))
2271 goto dma_err;
2272 if (++next_to_use == tpd_ring->count)
2273 next_to_use = 0;
2274 }
2275
2276 for (f = 0; f < nr_frags; f++) {
2277 const skb_frag_t *frag = &skb_shinfo(skb)->frags[f];
2278 u16 i, nseg;
2279
2280 buf_len = skb_frag_size(frag);
2281
2282 nseg = (buf_len + ATL1_MAX_TX_BUF_LEN - 1) /
2283 ATL1_MAX_TX_BUF_LEN;
2284 for (i = 0; i < nseg; i++) {
2285 buffer_info = &tpd_ring->buffer_info[next_to_use];
2286 BUG_ON(buffer_info->skb);
2287
2288 buffer_info->skb = NULL;
2289 buffer_info->length = (buf_len > ATL1_MAX_TX_BUF_LEN) ?
2290 ATL1_MAX_TX_BUF_LEN : buf_len;
2291 buf_len -= buffer_info->length;
2292 buffer_info->dma = skb_frag_dma_map(&adapter->pdev->dev,
2293 frag, i * ATL1_MAX_TX_BUF_LEN,
2294 buffer_info->length, DMA_TO_DEVICE);
2295 if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma))
2296 goto dma_err;
2297
2298 if (++next_to_use == tpd_ring->count)
2299 next_to_use = 0;
2300 }
2301 }
2302
2303 /* last tpd's buffer-info */
2304 buffer_info->skb = skb;
2305
2306 return 0;
2307
2308 dma_err:
2309 while (first_mapped != next_to_use) {
2310 buffer_info = &tpd_ring->buffer_info[first_mapped];
2311 dma_unmap_page(&adapter->pdev->dev,
2312 buffer_info->dma,
2313 buffer_info->length,
2314 DMA_TO_DEVICE);
> 2315 buffer_info->dma = NULL;
2316
> 2317 if (++first_mapped == tdp_ring->count)
2318 first_mapped = 0;
2319 }
2320 return -ENOMEM;
2321 }
2322
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Thomas Fourier <fourier.thomas@gmail.com> writes: > According to Shuah Khan[1], all `dma_map()` functions should be tested > before using the pointer. This patch checks for errors in `dma_map()` > calls and in case of failure, unmaps the previously dma_mapped regions > and returns an error. > > [1] https://events.static.linuxfound.org/sites/events/files/slides/Shuah_Khan_dma_map_error.pdf > > Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com> > --- > drivers/net/ethernet/atheros/atlx/atl1.c | 38 ++++++++++++++++++++++-- > 1 file changed, 36 insertions(+), 2 deletions(-) > [...] Hi Thomas, This doesn't seem to build. You can also see it here[1]. [1]: https://patchwork.kernel.org/project/netdevbpf/patch/20250612150542.85239-2-fourier.thomas@gmail.com/
On Thu, 12 Jun 2025 17:24:02 +0000 Charalampos Mitrodimas wrote: > This doesn't seem to build. You can also see it here[1]. > > [1]: https://patchwork.kernel.org/project/netdevbpf/patch/20250612150542.85239-2-fourier.thomas@gmail.com/ Please don't share links to the patchwork checks. Confirm its not a false positive and copy the output into the email. Patchwork checks are *not* a public CI for people to yolo their untested code into. Sharing links may give such impression.
According to Shuah Khan[1], all `dma_map()` functions should be tested
before using the pointer. This patch checks for errors after all `dma_map()`
calls.
In `atl1_alloc_rx_buffers()`, the buffer is deallocated ans marked as such.
In `atl1_tx_map()`, the arleady dma_mapped buffers are de-mapped and an error
is returned.
[1] https://events.static.linuxfound.org/sites/events/files/slides/Shuah_Khan_dma_map_error.pdf
Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
---
drivers/net/ethernet/atheros/atlx/atl1.c | 39 ++++++++++++++++++++++--
1 file changed, 37 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index cfdb546a09e7..d3cd51ccf621 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -1869,6 +1869,14 @@ static u16 atl1_alloc_rx_buffers(struct atl1_adapter *adapter)
buffer_info->dma = dma_map_page(&pdev->dev, page, offset,
adapter->rx_buffer_len,
DMA_FROM_DEVICE);
+ if (dma_mapping_error(&pdev->dev, buffer_info->dma)) {
+ buffer_info->alloced = 0;
+ buffer_info->skb = NULL;
+ buffer_info->dma = 0;
+ kfree_skb(skb);
+ adapter->soft_stats.rx_dropped++;
+ break;
+ }
rfd_desc->buffer_addr = cpu_to_le64(buffer_info->dma);
rfd_desc->buf_len = cpu_to_le16(adapter->rx_buffer_len);
rfd_desc->coalese = 0;
@@ -2183,7 +2191,7 @@ static int atl1_tx_csum(struct atl1_adapter *adapter, struct sk_buff *skb,
return 0;
}
-static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
+static int atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
struct tx_packet_desc *ptpd)
{
struct atl1_tpd_ring *tpd_ring = &adapter->tpd_ring;
@@ -2195,12 +2203,14 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
unsigned int f;
int retval;
u16 next_to_use;
+ u16 first_mapped;
u16 data_len;
u8 hdr_len;
buf_len -= skb->data_len;
nr_frags = skb_shinfo(skb)->nr_frags;
next_to_use = atomic_read(&tpd_ring->next_to_use);
+ first_mapped = next_to_use;
buffer_info = &tpd_ring->buffer_info[next_to_use];
BUG_ON(buffer_info->skb);
/* put skb in last TPD */
@@ -2216,6 +2226,8 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
buffer_info->dma = dma_map_page(&adapter->pdev->dev, page,
offset, hdr_len,
DMA_TO_DEVICE);
+ if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma))
+ goto dma_err;
if (++next_to_use == tpd_ring->count)
next_to_use = 0;
@@ -2242,6 +2254,8 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
page, offset,
buffer_info->length,
DMA_TO_DEVICE);
+ if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma))
+ goto dma_err;
if (++next_to_use == tpd_ring->count)
next_to_use = 0;
}
@@ -2254,6 +2268,8 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
buffer_info->dma = dma_map_page(&adapter->pdev->dev, page,
offset, buf_len,
DMA_TO_DEVICE);
+ if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma))
+ goto dma_err;
if (++next_to_use == tpd_ring->count)
next_to_use = 0;
}
@@ -2277,6 +2293,8 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
buffer_info->dma = skb_frag_dma_map(&adapter->pdev->dev,
frag, i * ATL1_MAX_TX_BUF_LEN,
buffer_info->length, DMA_TO_DEVICE);
+ if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma))
+ goto dma_err;
if (++next_to_use == tpd_ring->count)
next_to_use = 0;
@@ -2285,6 +2303,22 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
/* last tpd's buffer-info */
buffer_info->skb = skb;
+
+ return 0;
+
+ dma_err:
+ while (first_mapped != next_to_use) {
+ buffer_info = &tpd_ring->buffer_info[first_mapped];
+ dma_unmap_page(&adapter->pdev->dev,
+ buffer_info->dma,
+ buffer_info->length,
+ DMA_TO_DEVICE);
+ buffer_info->dma = 0;
+
+ if (++first_mapped == tpd_ring->count)
+ first_mapped = 0;
+ }
+ return -ENOMEM;
}
static void atl1_tx_queue(struct atl1_adapter *adapter, u16 count,
@@ -2419,7 +2453,8 @@ static netdev_tx_t atl1_xmit_frame(struct sk_buff *skb,
}
}
- atl1_tx_map(adapter, skb, ptpd);
+ if (atl1_tx_map(adapter, skb, ptpd))
+ return NETDEV_TX_BUSY;
atl1_tx_queue(adapter, count, ptpd);
atl1_update_mailbox(adapter);
return NETDEV_TX_OK;
--
2.43.0On Fri, 13 Jun 2025 11:54:08 +0200 Thomas Fourier wrote: > According to Shuah Khan[1], all `dma_map()` functions should be tested > before using the pointer. This patch checks for errors after all `dma_map()` > calls. Please read: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
According to Shuah Khan[1], all `dma_map()` functions should be tested
before using the pointer. This patch checks for errors after all
`dma_map()` calls.
In `atl1_alloc_rx_buffers()`, when the dma mapping fails,
the buffer is deallocated ans marked as such.
In `atl1_tx_map()`, the previously dma_mapped buffers are de-mapped and an
error is returned.
[1] https://events.static.linuxfound.org/sites/events/files/slides/Shuah_Khan_dma_map_error.pdf
Fixes: f3cc28c79760 ("Add Attansic L1 ethernet driver.")
Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
---
drivers/net/ethernet/atheros/atlx/atl1.c | 43 ++++++++++++++++++++++--
1 file changed, 40 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index cfdb546a09e7..dd0e01d3a023 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -1869,6 +1869,14 @@ static u16 atl1_alloc_rx_buffers(struct atl1_adapter *adapter)
buffer_info->dma = dma_map_page(&pdev->dev, page, offset,
adapter->rx_buffer_len,
DMA_FROM_DEVICE);
+ if (dma_mapping_error(&pdev->dev, buffer_info->dma)) {
+ buffer_info->alloced = 0;
+ buffer_info->skb = NULL;
+ buffer_info->dma = 0;
+ kfree_skb(skb);
+ adapter->soft_stats.rx_dropped++;
+ break;
+ }
rfd_desc->buffer_addr = cpu_to_le64(buffer_info->dma);
rfd_desc->buf_len = cpu_to_le16(adapter->rx_buffer_len);
rfd_desc->coalese = 0;
@@ -2183,8 +2191,8 @@ static int atl1_tx_csum(struct atl1_adapter *adapter, struct sk_buff *skb,
return 0;
}
-static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
- struct tx_packet_desc *ptpd)
+static int atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
+ struct tx_packet_desc *ptpd)
{
struct atl1_tpd_ring *tpd_ring = &adapter->tpd_ring;
struct atl1_buffer *buffer_info;
@@ -2194,6 +2202,7 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
unsigned int nr_frags;
unsigned int f;
int retval;
+ u16 first_mapped;
u16 next_to_use;
u16 data_len;
u8 hdr_len;
@@ -2201,6 +2210,7 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
buf_len -= skb->data_len;
nr_frags = skb_shinfo(skb)->nr_frags;
next_to_use = atomic_read(&tpd_ring->next_to_use);
+ first_mapped = next_to_use;
buffer_info = &tpd_ring->buffer_info[next_to_use];
BUG_ON(buffer_info->skb);
/* put skb in last TPD */
@@ -2216,6 +2226,8 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
buffer_info->dma = dma_map_page(&adapter->pdev->dev, page,
offset, hdr_len,
DMA_TO_DEVICE);
+ if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma))
+ goto dma_err;
if (++next_to_use == tpd_ring->count)
next_to_use = 0;
@@ -2242,6 +2254,9 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
page, offset,
buffer_info->length,
DMA_TO_DEVICE);
+ if (dma_mapping_error(&adapter->pdev->dev,
+ buffer_info->dma))
+ goto dma_err;
if (++next_to_use == tpd_ring->count)
next_to_use = 0;
}
@@ -2254,6 +2269,8 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
buffer_info->dma = dma_map_page(&adapter->pdev->dev, page,
offset, buf_len,
DMA_TO_DEVICE);
+ if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma))
+ goto dma_err;
if (++next_to_use == tpd_ring->count)
next_to_use = 0;
}
@@ -2277,6 +2294,9 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
buffer_info->dma = skb_frag_dma_map(&adapter->pdev->dev,
frag, i * ATL1_MAX_TX_BUF_LEN,
buffer_info->length, DMA_TO_DEVICE);
+ if (dma_mapping_error(&adapter->pdev->dev,
+ buffer_info->dma))
+ goto dma_err;
if (++next_to_use == tpd_ring->count)
next_to_use = 0;
@@ -2285,6 +2305,22 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
/* last tpd's buffer-info */
buffer_info->skb = skb;
+
+ return 0;
+
+ dma_err:
+ while (first_mapped != next_to_use) {
+ buffer_info = &tpd_ring->buffer_info[first_mapped];
+ dma_unmap_page(&adapter->pdev->dev,
+ buffer_info->dma,
+ buffer_info->length,
+ DMA_TO_DEVICE);
+ buffer_info->dma = 0;
+
+ if (++first_mapped == tpd_ring->count)
+ first_mapped = 0;
+ }
+ return -ENOMEM;
}
static void atl1_tx_queue(struct atl1_adapter *adapter, u16 count,
@@ -2419,7 +2455,8 @@ static netdev_tx_t atl1_xmit_frame(struct sk_buff *skb,
}
}
- atl1_tx_map(adapter, skb, ptpd);
+ if (atl1_tx_map(adapter, skb, ptpd))
+ return NETDEV_TX_BUSY;
atl1_tx_queue(adapter, count, ptpd);
atl1_update_mailbox(adapter);
return NETDEV_TX_OK;
--
2.43.0On Mon, 16 Jun 2025 15:59:55 +0200 Thomas Fourier wrote:
> According to Shuah Khan[1], all `dma_map()` functions should be tested
> before using the pointer. This patch checks for errors after all
> `dma_map()` calls.
The link and reference to Shuah's presentation is not necessary.
Just say that the DMA functions can fail so we need the error check.
> In `atl1_alloc_rx_buffers()`, when the dma mapping fails,
> the buffer is deallocated ans marked as such.
>
> In `atl1_tx_map()`, the previously dma_mapped buffers are de-mapped and an
> error is returned.
>
> [1] https://events.static.linuxfound.org/sites/events/files/slides/Shuah_Khan_dma_map_error.pdf
>
> Fixes: f3cc28c79760 ("Add Attansic L1 ethernet driver.")
> Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
Please don't send the new versions in reply to old ones.
For those of us who use their inbox as a FIFO of pending submissions
it messes up ordering.
> diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
> index cfdb546a09e7..dd0e01d3a023 100644
> --- a/drivers/net/ethernet/atheros/atlx/atl1.c
> +++ b/drivers/net/ethernet/atheros/atlx/atl1.c
> @@ -1869,6 +1869,14 @@ static u16 atl1_alloc_rx_buffers(struct atl1_adapter *adapter)
> buffer_info->dma = dma_map_page(&pdev->dev, page, offset,
> adapter->rx_buffer_len,
> DMA_FROM_DEVICE);
> + if (dma_mapping_error(&pdev->dev, buffer_info->dma)) {
> + buffer_info->alloced = 0;
> + buffer_info->skb = NULL;
> + buffer_info->dma = 0;
maybe you could move the map call a little further up, before all the
buffer_info fields are populated? So we dont have to clean them up?
> + kfree_skb(skb);
> + adapter->soft_stats.rx_dropped++;
> + break;
> + }
> rfd_desc->buffer_addr = cpu_to_le64(buffer_info->dma);
> rfd_desc->buf_len = cpu_to_le16(adapter->rx_buffer_len);
> rfd_desc->coalese = 0;
> @@ -2183,8 +2191,8 @@ static int atl1_tx_csum(struct atl1_adapter *adapter, struct sk_buff *skb,
> return 0;
> }
>
> -static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
> - struct tx_packet_desc *ptpd)
> +static int atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
> + struct tx_packet_desc *ptpd)
> {
> struct atl1_tpd_ring *tpd_ring = &adapter->tpd_ring;
> struct atl1_buffer *buffer_info;
> @@ -2194,6 +2202,7 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
> unsigned int nr_frags;
> unsigned int f;
> int retval;
> + u16 first_mapped;
> u16 next_to_use;
> u16 data_len;
> u8 hdr_len;
> @@ -2201,6 +2210,7 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
> buf_len -= skb->data_len;
> nr_frags = skb_shinfo(skb)->nr_frags;
> next_to_use = atomic_read(&tpd_ring->next_to_use);
> + first_mapped = next_to_use;
> buffer_info = &tpd_ring->buffer_info[next_to_use];
> BUG_ON(buffer_info->skb);
> /* put skb in last TPD */
> @@ -2216,6 +2226,8 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
> buffer_info->dma = dma_map_page(&adapter->pdev->dev, page,
> offset, hdr_len,
> DMA_TO_DEVICE);
> + if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma))
> + goto dma_err;
>
> if (++next_to_use == tpd_ring->count)
> next_to_use = 0;
> @@ -2242,6 +2254,9 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
> page, offset,
> buffer_info->length,
> DMA_TO_DEVICE);
> + if (dma_mapping_error(&adapter->pdev->dev,
> + buffer_info->dma))
> + goto dma_err;
> if (++next_to_use == tpd_ring->count)
> next_to_use = 0;
> }
> @@ -2254,6 +2269,8 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
> buffer_info->dma = dma_map_page(&adapter->pdev->dev, page,
> offset, buf_len,
> DMA_TO_DEVICE);
> + if (dma_mapping_error(&adapter->pdev->dev, buffer_info->dma))
> + goto dma_err;
> if (++next_to_use == tpd_ring->count)
> next_to_use = 0;
> }
> @@ -2277,6 +2294,9 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
> buffer_info->dma = skb_frag_dma_map(&adapter->pdev->dev,
> frag, i * ATL1_MAX_TX_BUF_LEN,
> buffer_info->length, DMA_TO_DEVICE);
> + if (dma_mapping_error(&adapter->pdev->dev,
> + buffer_info->dma))
> + goto dma_err;
>
> if (++next_to_use == tpd_ring->count)
> next_to_use = 0;
> @@ -2285,6 +2305,22 @@ static void atl1_tx_map(struct atl1_adapter *adapter, struct sk_buff *skb,
>
> /* last tpd's buffer-info */
> buffer_info->skb = skb;
> +
> + return 0;
> +
> + dma_err:
> + while (first_mapped != next_to_use) {
> + buffer_info = &tpd_ring->buffer_info[first_mapped];
> + dma_unmap_page(&adapter->pdev->dev,
> + buffer_info->dma,
> + buffer_info->length,
> + DMA_TO_DEVICE);
> + buffer_info->dma = 0;
> +
> + if (++first_mapped == tpd_ring->count)
> + first_mapped = 0;
> + }
> + return -ENOMEM;
> }
>
> static void atl1_tx_queue(struct atl1_adapter *adapter, u16 count,
> @@ -2419,7 +2455,8 @@ static netdev_tx_t atl1_xmit_frame(struct sk_buff *skb,
> }
> }
>
> - atl1_tx_map(adapter, skb, ptpd);
> + if (atl1_tx_map(adapter, skb, ptpd))
> + return NETDEV_TX_BUSY;
You should drop the packet. Occasional packet loss is better than
having a packet that we can't map for some platform reasons blocking
the queue
> atl1_tx_queue(adapter, count, ptpd);
> atl1_update_mailbox(adapter);
> return NETDEV_TX_OK;
On Mon, 16 Jun 2025 15:59:55 +0200 Thomas Fourier wrote: > According to Shuah Khan[1] Sorry for a non-technical question -- are you part of some outreach / training program? The presentation you linked is from 2013, I wonder what made you pick up this particular task.
On 16/06/2025 19:46, Jakub Kicinski wrote: > On Mon, 16 Jun 2025 15:59:55 +0200 Thomas Fourier wrote: >> According to Shuah Khan[1] > Sorry for a non-technical question -- are you part of some outreach / > training program? The presentation you linked is from 2013, I wonder > what made you pick up this particular task. I am doing a master thesis on static analysis and I am writing a checker with Smatch to test if error codes are well-checked. My supervisor suggested that I look at DMA mapping errors checks to see if people were interested in such patches and because they are quite easy to statically assert.
On Tue, 17 Jun 2025 17:47:50 +0200 Thomas Fourier wrote: > On 16/06/2025 19:46, Jakub Kicinski wrote: > > On Mon, 16 Jun 2025 15:59:55 +0200 Thomas Fourier wrote: > >> According to Shuah Khan[1] > > Sorry for a non-technical question -- are you part of some outreach / > > training program? The presentation you linked is from 2013, I wonder > > what made you pick up this particular task. > > I am doing a master thesis on static analysis and I am writing a checker > with I see. > Smatch to test if error codes are well-checked. My supervisor suggested > that > > I look at DMA mapping errors checks to see if people were interested in such > > patches and because they are quite easy to statically assert. Given the community's mixed experience with researches, would you be willing to share the name of the institution ?
© 2016 - 2025 Red Hat, Inc.