[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <0d4d2010-79e3-47d3-bcef-d6140fc68596@ti.com>
Date: Mon, 17 Mar 2025 15:43:56 +0530
From: "Malladi, Meghana" <m-malladi@...com>
To: Dan Carpenter <dan.carpenter@...aro.org>
CC: <netdev@...r.kernel.org>, Roger Quadros <rogerq@...nel.org>
Subject: Re: [EXTERNAL] Re: [bug report] net: ti: icssg-prueth: Add XDP
support
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
>
Powered by blists - more mailing lists