lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ