* [bug report] net: ti: icssg-prueth: Add XDP support
@ 2025-03-14 10:50 Dan Carpenter
2025-03-14 20:31 ` Roger Quadros
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2025-03-14 10:50 UTC (permalink / raw)
To: Roger Quadros; +Cc: netdev
Hello Roger Quadros,
Commit 62aa3246f462 ("net: ti: icssg-prueth: Add XDP support") from
Mar 5, 2025 (linux-next), leads to the following Smatch static
checker warning:
drivers/net/ethernet/ti/icssg/icssg_common.c:635 emac_xmit_xdp_frame()
error: we previously assumed 'first_desc' could be null (see line 584)
drivers/net/ethernet/ti/icssg/icssg_common.c
563 u32 emac_xmit_xdp_frame(struct prueth_emac *emac,
564 struct xdp_frame *xdpf,
565 struct page *page,
566 unsigned int q_idx)
567 {
568 struct cppi5_host_desc_t *first_desc;
569 struct net_device *ndev = emac->ndev;
570 struct prueth_tx_chn *tx_chn;
571 dma_addr_t desc_dma, buf_dma;
572 struct prueth_swdata *swdata;
573 u32 *epib;
574 int ret;
575
576 if (q_idx >= PRUETH_MAX_TX_QUEUES) {
577 netdev_err(ndev, "xdp tx: invalid q_id %d\n", q_idx);
578 return ICSSG_XDP_CONSUMED; /* drop */
Do we need to free something on this path?
579 }
580
581 tx_chn = &emac->tx_chns[q_idx];
582
583 first_desc = k3_cppi_desc_pool_alloc(tx_chn->desc_pool);
584 if (!first_desc) {
585 netdev_dbg(ndev, "xdp tx: failed to allocate descriptor\n");
586 goto drop_free_descs; /* drop */
^^^^^^^^^^^^^^^^^^^^
This will dereference first_desc and crash.
587 }
588
589 if (page) { /* already DMA mapped by page_pool */
590 buf_dma = page_pool_get_dma_addr(page);
591 buf_dma += xdpf->headroom + sizeof(struct xdp_frame);
592 } else { /* Map the linear buffer */
593 buf_dma = dma_map_single(tx_chn->dma_dev, xdpf->data, xdpf->len, DMA_TO_DEVICE);
594 if (dma_mapping_error(tx_chn->dma_dev, buf_dma)) {
595 netdev_err(ndev, "xdp tx: failed to map data buffer\n");
596 goto drop_free_descs; /* drop */
597 }
598 }
599
600 cppi5_hdesc_init(first_desc, CPPI5_INFO0_HDESC_EPIB_PRESENT,
601 PRUETH_NAV_PS_DATA_SIZE);
602 cppi5_hdesc_set_pkttype(first_desc, 0);
603 epib = first_desc->epib;
604 epib[0] = 0;
605 epib[1] = 0;
606
607 /* set dst tag to indicate internal qid at the firmware which is at
608 * bit8..bit15. bit0..bit7 indicates port num for directed
609 * packets in case of switch mode operation
610 */
611 cppi5_desc_set_tags_ids(&first_desc->hdr, 0, (emac->port_id | (q_idx << 8)));
612 k3_udma_glue_tx_dma_to_cppi5_addr(tx_chn->tx_chn, &buf_dma);
613 cppi5_hdesc_attach_buf(first_desc, buf_dma, xdpf->len, buf_dma, xdpf->len);
614 swdata = cppi5_hdesc_get_swdata(first_desc);
615 if (page) {
616 swdata->type = PRUETH_SWDATA_PAGE;
617 swdata->data.page = page;
618 } else {
619 swdata->type = PRUETH_SWDATA_XDPF;
620 swdata->data.xdpf = xdpf;
621 }
622
623 cppi5_hdesc_set_pktlen(first_desc, xdpf->len);
624 desc_dma = k3_cppi_desc_pool_virt2dma(tx_chn->desc_pool, first_desc);
625
626 ret = k3_udma_glue_push_tx_chn(tx_chn->tx_chn, first_desc, desc_dma);
627 if (ret) {
628 netdev_err(ndev, "xdp tx: push failed: %d\n", ret);
629 goto drop_free_descs;
630 }
631
632 return ICSSG_XDP_TX;
633
634 drop_free_descs:
635 prueth_xmit_free(tx_chn, first_desc);
636 return ICSSG_XDP_CONSUMED;
637 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] net: ti: icssg-prueth: Add XDP support
2025-03-14 10:50 [bug report] net: ti: icssg-prueth: Add XDP support Dan Carpenter
@ 2025-03-14 20:31 ` Roger Quadros
2025-03-17 10:13 ` [EXTERNAL] " Malladi, Meghana
0 siblings, 1 reply; 3+ messages in thread
From: Roger Quadros @ 2025-03-14 20:31 UTC (permalink / raw)
To: Dan Carpenter, Malladi, Meghana; +Cc: netdev
+Meghana,
On 14/03/2025 12:50, Dan Carpenter wrote:
> Hello Roger Quadros,
>
> Commit 62aa3246f462 ("net: ti: icssg-prueth: Add XDP support") from
> Mar 5, 2025 (linux-next), leads to the following Smatch static
> checker warning:
>
> drivers/net/ethernet/ti/icssg/icssg_common.c:635 emac_xmit_xdp_frame()
> error: we previously assumed 'first_desc' could be null (see line 584)
>
> drivers/net/ethernet/ti/icssg/icssg_common.c
> 563 u32 emac_xmit_xdp_frame(struct prueth_emac *emac,
> 564 struct xdp_frame *xdpf,
> 565 struct page *page,
> 566 unsigned int q_idx)
> 567 {
> 568 struct cppi5_host_desc_t *first_desc;
> 569 struct net_device *ndev = emac->ndev;
> 570 struct prueth_tx_chn *tx_chn;
> 571 dma_addr_t desc_dma, buf_dma;
> 572 struct prueth_swdata *swdata;
> 573 u32 *epib;
> 574 int ret;
> 575
> 576 if (q_idx >= PRUETH_MAX_TX_QUEUES) {
> 577 netdev_err(ndev, "xdp tx: invalid q_id %d\n", q_idx);
> 578 return ICSSG_XDP_CONSUMED; /* drop */
>
> Do we need to free something on this path?
>
> 579 }
> 580
> 581 tx_chn = &emac->tx_chns[q_idx];
> 582
> 583 first_desc = k3_cppi_desc_pool_alloc(tx_chn->desc_pool);
> 584 if (!first_desc) {
> 585 netdev_dbg(ndev, "xdp tx: failed to allocate descriptor\n");
> 586 goto drop_free_descs; /* drop */
> ^^^^^^^^^^^^^^^^^^^^
> This will dereference first_desc and crash.
>
> 587 }
> 588
> 589 if (page) { /* already DMA mapped by page_pool */
> 590 buf_dma = page_pool_get_dma_addr(page);
> 591 buf_dma += xdpf->headroom + sizeof(struct xdp_frame);
> 592 } else { /* Map the linear buffer */
> 593 buf_dma = dma_map_single(tx_chn->dma_dev, xdpf->data, xdpf->len, DMA_TO_DEVICE);
> 594 if (dma_mapping_error(tx_chn->dma_dev, buf_dma)) {
> 595 netdev_err(ndev, "xdp tx: failed to map data buffer\n");
> 596 goto drop_free_descs; /* drop */
> 597 }
> 598 }
> 599
> 600 cppi5_hdesc_init(first_desc, CPPI5_INFO0_HDESC_EPIB_PRESENT,
> 601 PRUETH_NAV_PS_DATA_SIZE);
> 602 cppi5_hdesc_set_pkttype(first_desc, 0);
> 603 epib = first_desc->epib;
> 604 epib[0] = 0;
> 605 epib[1] = 0;
> 606
> 607 /* set dst tag to indicate internal qid at the firmware which is at
> 608 * bit8..bit15. bit0..bit7 indicates port num for directed
> 609 * packets in case of switch mode operation
> 610 */
> 611 cppi5_desc_set_tags_ids(&first_desc->hdr, 0, (emac->port_id | (q_idx << 8)));
> 612 k3_udma_glue_tx_dma_to_cppi5_addr(tx_chn->tx_chn, &buf_dma);
> 613 cppi5_hdesc_attach_buf(first_desc, buf_dma, xdpf->len, buf_dma, xdpf->len);
> 614 swdata = cppi5_hdesc_get_swdata(first_desc);
> 615 if (page) {
> 616 swdata->type = PRUETH_SWDATA_PAGE;
> 617 swdata->data.page = page;
> 618 } else {
> 619 swdata->type = PRUETH_SWDATA_XDPF;
> 620 swdata->data.xdpf = xdpf;
> 621 }
> 622
> 623 cppi5_hdesc_set_pktlen(first_desc, xdpf->len);
> 624 desc_dma = k3_cppi_desc_pool_virt2dma(tx_chn->desc_pool, first_desc);
> 625
> 626 ret = k3_udma_glue_push_tx_chn(tx_chn->tx_chn, first_desc, desc_dma);
> 627 if (ret) {
> 628 netdev_err(ndev, "xdp tx: push failed: %d\n", ret);
> 629 goto drop_free_descs;
> 630 }
> 631
> 632 return ICSSG_XDP_TX;
> 633
> 634 drop_free_descs:
> 635 prueth_xmit_free(tx_chn, first_desc);
> 636 return ICSSG_XDP_CONSUMED;
> 637 }
>
>
> regards,
> dan carpenter
--
cheers,
-roger
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [EXTERNAL] Re: [bug report] net: ti: icssg-prueth: Add XDP support
2025-03-14 20:31 ` Roger Quadros
@ 2025-03-17 10:13 ` Malladi, Meghana
0 siblings, 0 replies; 3+ messages in thread
From: Malladi, Meghana @ 2025-03-17 10:13 UTC (permalink / raw)
To: Dan Carpenter; +Cc: netdev, Roger Quadros
Hi Dan,
On 3/15/2025 2:01 AM, Roger Quadros wrote:
> +Meghana, On 14/03/2025 12: 50, Dan Carpenter wrote: > Hello Roger
> Quadros, > > Commit 62aa3246f462 ("net: ti: icssg-prueth: Add XDP
> support") from > Mar 5, 2025 (linux-next), leads to the following Smatch
> static > checker warning:
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source
> of this email and know the content is safe.
> Report Suspicious
> <https://us-phishalarm-ewt.proofpoint.com/EWT/v1/G3vK!
> updgPZavlq17YEXEXDMjX3l3H00pWvtYT_4pQscvyXpIdw2OveCVYQibZIdWxZw5GR1hxtmi6vr2_ObXj1T_qBc3C0dxD5U$>
> ZjQcmQRYFpfptBannerEnd
>
> +Meghana,
>
> On 14/03/2025 12:50, Dan Carpenter wrote:
>> Hello Roger Quadros,
>>
>> Commit 62aa3246f462 ("net: ti: icssg-prueth: Add XDP support") from
>> Mar 5, 2025 (linux-next), leads to the following Smatch static
>> checker warning:
>>
>> drivers/net/ethernet/ti/icssg/icssg_common.c:635 emac_xmit_xdp_frame()
>> error: we previously assumed 'first_desc' could be null (see line 584)
>>
>> drivers/net/ethernet/ti/icssg/icssg_common.c
>> 563 u32 emac_xmit_xdp_frame(struct prueth_emac *emac,
>> 564 struct xdp_frame *xdpf,
>> 565 struct page *page,
>> 566 unsigned int q_idx)
>> 567 {
>> 568 struct cppi5_host_desc_t *first_desc;
>> 569 struct net_device *ndev = emac->ndev;
>> 570 struct prueth_tx_chn *tx_chn;
>> 571 dma_addr_t desc_dma, buf_dma;
>> 572 struct prueth_swdata *swdata;
>> 573 u32 *epib;
>> 574 int ret;
>> 575
>> 576 if (q_idx >= PRUETH_MAX_TX_QUEUES) {
>> 577 netdev_err(ndev, "xdp tx: invalid q_id %d\n", q_idx);
>> 578 return ICSSG_XDP_CONSUMED; /* drop */
>>
>> Do we need to free something on this path?
>>
Freeing the page is handled by the caller of this function based of the
return type.
>> 579 }
>> 580
>> 581 tx_chn = &emac->tx_chns[q_idx];
>> 582
>> 583 first_desc = k3_cppi_desc_pool_alloc(tx_chn->desc_pool);
>> 584 if (!first_desc) {
>> 585 netdev_dbg(ndev, "xdp tx: failed to allocate descriptor\n");
>> 586 goto drop_free_descs; /* drop */
>> ^^^^^^^^^^^^^^^^^^^^
>> This will dereference first_desc and crash.
>>
Thanks for catching this bug, will post fix for this shortly.
>> 587 }
>> 588
>> 589 if (page) { /* already DMA mapped by page_pool */
>> 590 buf_dma = page_pool_get_dma_addr(page);
>> 591 buf_dma += xdpf->headroom + sizeof(struct xdp_frame);
>> 592 } else { /* Map the linear buffer */
>> 593 buf_dma = dma_map_single(tx_chn->dma_dev, xdpf->data, xdpf->len, DMA_TO_DEVICE);
>> 594 if (dma_mapping_error(tx_chn->dma_dev, buf_dma)) {
>> 595 netdev_err(ndev, "xdp tx: failed to map data buffer\n");
>> 596 goto drop_free_descs; /* drop */
>> 597 }
>> 598 }
>> 599
>> 600 cppi5_hdesc_init(first_desc, CPPI5_INFO0_HDESC_EPIB_PRESENT,
>> 601 PRUETH_NAV_PS_DATA_SIZE);
>> 602 cppi5_hdesc_set_pkttype(first_desc, 0);
>> 603 epib = first_desc->epib;
>> 604 epib[0] = 0;
>> 605 epib[1] = 0;
>> 606
>> 607 /* set dst tag to indicate internal qid at the firmware which is at
>> 608 * bit8..bit15. bit0..bit7 indicates port num for directed
>> 609 * packets in case of switch mode operation
>> 610 */
>> 611 cppi5_desc_set_tags_ids(&first_desc->hdr, 0, (emac->port_id | (q_idx << 8)));
>> 612 k3_udma_glue_tx_dma_to_cppi5_addr(tx_chn->tx_chn, &buf_dma);
>> 613 cppi5_hdesc_attach_buf(first_desc, buf_dma, xdpf->len, buf_dma, xdpf->len);
>> 614 swdata = cppi5_hdesc_get_swdata(first_desc);
>> 615 if (page) {
>> 616 swdata->type = PRUETH_SWDATA_PAGE;
>> 617 swdata->data.page = page;
>> 618 } else {
>> 619 swdata->type = PRUETH_SWDATA_XDPF;
>> 620 swdata->data.xdpf = xdpf;
>> 621 }
>> 622
>> 623 cppi5_hdesc_set_pktlen(first_desc, xdpf->len);
>> 624 desc_dma = k3_cppi_desc_pool_virt2dma(tx_chn->desc_pool, first_desc);
>> 625
>> 626 ret = k3_udma_glue_push_tx_chn(tx_chn->tx_chn, first_desc, desc_dma);
>> 627 if (ret) {
>> 628 netdev_err(ndev, "xdp tx: push failed: %d\n", ret);
>> 629 goto drop_free_descs;
>> 630 }
>> 631
>> 632 return ICSSG_XDP_TX;
>> 633
>> 634 drop_free_descs:
>> 635 prueth_xmit_free(tx_chn, first_desc);
>> 636 return ICSSG_XDP_CONSUMED;
>> 637 }
>>
>>
>> regards,
>> dan carpenter
>
> --
> cheers,
> -roger
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-03-17 10:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-14 10:50 [bug report] net: ti: icssg-prueth: Add XDP support Dan Carpenter
2025-03-14 20:31 ` Roger Quadros
2025-03-17 10:13 ` [EXTERNAL] " Malladi, Meghana
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).