netdev - Re: [PATCH net-next V4 5/9] eth: fbnic: Add XDP pass, drop, abort support

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: <aKcWe3bm3wQqlfdx@stanley.mountain>
Date: Thu, 21 Aug 2025 15:52:11 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Mohsin Bashir <mohsin.bashr@...il.com>
Cc: netdev@...r.kernel.org, aleksander.lobakin@...el.com,
	alexanderduyck@...com, andrew+netdev@...n.ch, ast@...nel.org,
	bpf@...r.kernel.org, corbet@....net, daniel@...earbox.net,
	davem@...emloft.net, edumazet@...gle.com, hawk@...nel.org,
	horms@...nel.org, john.fastabend@...il.com, kernel-team@...a.com,
	kuba@...nel.org, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org, pabeni@...hat.com, sdf@...ichev.me,
	vadim.fedorenko@...ux.dev
Subject: Re: [PATCH net-next V4 5/9] eth: fbnic: Add XDP pass, drop, abort
 support

On Wed, Aug 13, 2025 at 03:13:15PM -0700, Mohsin Bashir wrote:
> @@ -1251,6 +1293,7 @@ static void fbnic_free_napi_vector(struct fbnic_net *fbn,
>  	}
>  
>  	for (j = 0; j < nv->rxt_count; j++, i++) {
> +		xdp_rxq_info_unreg(&nv->qt[i].xdp_rxq);
>  		fbnic_remove_rx_ring(fbn, &nv->qt[i].sub0);
>  		fbnic_remove_rx_ring(fbn, &nv->qt[i].sub1);
>  		fbnic_remove_rx_ring(fbn, &nv->qt[i].cmpl);
> @@ -1423,6 +1466,11 @@ static int fbnic_alloc_napi_vector(struct fbnic_dev *fbd, struct fbnic_net *fbn,
>  		fbnic_ring_init(&qt->cmpl, db, rxq_idx, FBNIC_RING_F_STATS);
>  		fbn->rx[rxq_idx] = &qt->cmpl;
>  
> +		err = xdp_rxq_info_reg(&qt->xdp_rxq, fbn->netdev, rxq_idx,
> +				       nv->napi.napi_id);
> +		if (err)
> +			goto free_ring_cur_qt;
> +
>  		/* Update Rx queue index */
>  		rxt_count--;
>  		rxq_idx += v_count;
> @@ -1433,6 +1481,25 @@ static int fbnic_alloc_napi_vector(struct fbnic_dev *fbd, struct fbnic_net *fbn,
>  
>  	return 0;
>  
> +	while (rxt_count < nv->rxt_count) {
               ^^^^^^^^^^^^^^^^^^^^^^^^^
This should be <= otherwise it won't free enough.  Then qt will point to
the wrong thing and the next loop will crash.

The loops in this function are mind bendingly complicated.  It might be
easiter to write them as:

	for (i = 0; i < nv->txt_count; i++) {
		qt = &nv->qt[i];
		...
	}

	for (i = 0; i < nv->rxt_count; i++) {
		qt = &nv->qt[txt_count + i];
		...
	}

Generally, I would just unwind the partial loop before the goto instead
of doing a jump to the middle of the goto.  It's more lines of code, but
I'm stupid, so I prefer code which is easy even if it's longer.

regards,
dan carpenter

> +		qt--;
> +
> +		xdp_rxq_info_unreg(&qt->xdp_rxq);
> +free_ring_cur_qt:
> +		fbnic_remove_rx_ring(fbn, &qt->sub0);
> +		fbnic_remove_rx_ring(fbn, &qt->sub1);
> +		fbnic_remove_rx_ring(fbn, &qt->cmpl);
> +		rxt_count++;
> +	}
> +	while (txt_count < nv->txt_count) {
> +		qt--;
> +
> +		fbnic_remove_tx_ring(fbn, &qt->sub0);
> +		fbnic_remove_tx_ring(fbn, &qt->cmpl);
> +
> +		txt_count++;
> +	}
> +	fbnic_napi_free_irq(fbd, nv);
>  pp_destroy:
>  	page_pool_destroy(nv->page_pool);
>  napi_del: