> Hello Lorenzo Bianconi,
Hi Dan,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch c41ced023a98: "net: mvneta: add frags support to XDP_TX"
> from Jan 21, 2022, leads to the following Smatch complaint:
>
> drivers/net/ethernet/marvell/mvneta.c:2145 mvneta_xdp_submit_frame()
> warn: variable dereferenced before check 'tx_desc' (see line 2136)
>
> drivers/net/ethernet/marvell/mvneta.c
> 2080 static int
> 2081 mvneta_xdp_submit_frame(struct mvneta_port *pp, struct mvneta_tx_queue *txq,
> 2082 struct xdp_frame *xdpf, int *nxmit_byte, bool dma_map)
> 2083 {
> 2084 struct skb_shared_info *sinfo = xdp_get_shared_info_from_frame(xdpf);
> 2085 struct device *dev = pp->dev->dev.parent;
> 2086 struct mvneta_tx_desc *tx_desc = NULL;
> 2087 int i, num_frames = 1;
> 2088 struct page *page;
> 2089
> 2090 if (unlikely(xdp_frame_has_frags(xdpf)))
> 2091 num_frames += sinfo->nr_frags;
> 2092
> 2093 if (txq->count + num_frames >= txq->size)
> 2094 return MVNETA_XDP_DROPPED;
> 2095
> 2096 for (i = 0; i < num_frames; i++) {
>
> We know num_frames is non-zero
>
> 2097 struct mvneta_tx_buf *buf = &txq->buf[txq->txq_put_index];
> 2098 skb_frag_t *frag = NULL;
> 2099 int len = xdpf->len;
> 2100 dma_addr_t dma_addr;
> 2101
> 2102 if (unlikely(i)) { /* paged area */
> 2103 frag = &sinfo->frags[i - 1];
> 2104 len = skb_frag_size(frag);
> 2105 }
> 2106
> 2107 tx_desc = mvneta_txq_next_desc_get(txq);
> 2108 if (dma_map) {
> 2109 /* ndo_xdp_xmit */
> 2110 void *data;
> 2111
> 2112 data = unlikely(frag) ? skb_frag_address(frag)
> 2113 : xdpf->data;
> 2114 dma_addr = dma_map_single(dev, data, len,
> 2115 DMA_TO_DEVICE);
> 2116 if (dma_mapping_error(dev, dma_addr)) {
> 2117 mvneta_txq_desc_put(txq);
> 2118 goto unmap;
> 2119 }
> 2120
> 2121 buf->type = MVNETA_TYPE_XDP_NDO;
> 2122 } else {
> 2123 page = unlikely(frag) ? skb_frag_page(frag)
> 2124 : virt_to_page(xdpf->data);
> 2125 dma_addr = page_pool_get_dma_addr(page);
> 2126 if (unlikely(frag))
> 2127 dma_addr += skb_frag_off(frag);
> 2128 else
> 2129 dma_addr += sizeof(*xdpf) + xdpf->headroom;
> 2130 dma_sync_single_for_device(dev, dma_addr, len,
> 2131 DMA_BIDIRECTIONAL);
> 2132 buf->type = MVNETA_TYPE_XDP_TX;
> 2133 }
> 2134 buf->xdpf = unlikely(i) ? NULL : xdpf;
> 2135
> 2136 tx_desc->command = unlikely(i) ? 0 : MVNETA_TXD_F_DESC;
> ^^^^^^^^^^^^^^^^
> Dereferenced
>
> 2137 tx_desc->buf_phys_addr = dma_addr;
> 2138 tx_desc->data_size = len;
> 2139 *nxmit_byte += len;
> 2140
> 2141 mvneta_txq_inc_put(txq);
> 2142 }
> 2143
> 2144 /*last descriptor */
> 2145 if (likely(tx_desc))
since num_frames is at least 1, I guess we can drop this if condition.
Regards,
Lorenzo
> ^^^^^^^
> Checked after dereference.
>
> 2146 tx_desc->command |= MVNETA_TXD_L_DESC | MVNETA_TXD_Z_PAD;
> 2147
> 2148 txq->pending += num_frames;
> 2149 txq->count += num_frames;
> 2150
> 2151 return MVNETA_XDP_TX;
> 2152
> 2153 unmap:
> 2154 for (i--; i >= 0; i--) {
> 2155 mvneta_txq_desc_put(txq);
> 2156 tx_desc = txq->descs + txq->next_desc_to_proc;
> 2157 dma_unmap_single(dev, tx_desc->buf_phys_addr,
> 2158 tx_desc->data_size,
>
> regards,
> dan carpenter
Attachment:
signature.asc
Description: PGP signature
