public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] net: avoid nul-deref trying to bind mp to incapable device
@ 2026-04-04  0:19 Jakub Kicinski
  2026-04-04 22:43 ` Daniel Borkmann
  2026-04-06 23:55 ` Mina Almasry
  0 siblings, 2 replies; 3+ messages in thread
From: Jakub Kicinski @ 2026-04-04  0:19 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, Jakub Kicinski,
	sdf, almasrymina, daniel

Sashiko points out that we use qops in __net_mp_open_rxq()
but never validate they are null. This was introduced when
check was moved from netdev_rx_queue_restart().

Look at ops directly instead of the locking config.
qops imply netdev_need_ops_lock(). We used netdev_need_ops_lock()
initially to signify that the real_num_rx_queues check below
is safe without rtnl_lock, but I'm not sure if this is actually
clear to most people, anyway.

Fixes: da7772a2b4ad ("net: move mp->rx_page_size validation to __net_mp_open_rxq()")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: sdf@fomichev.me
CC: almasrymina@google.com
CC: daniel@iogearbox.net
---
 net/core/netdev_rx_queue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/netdev_rx_queue.c b/net/core/netdev_rx_queue.c
index 668a90658f25..05fd2875d725 100644
--- a/net/core/netdev_rx_queue.c
+++ b/net/core/netdev_rx_queue.c
@@ -117,7 +117,7 @@ int __net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
 	struct netdev_rx_queue *rxq;
 	int ret;
 
-	if (!netdev_need_ops_lock(dev))
+	if (!qops)
 		return -EOPNOTSUPP;
 
 	if (rxq_idx >= dev->real_num_rx_queues) {
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: avoid nul-deref trying to bind mp to incapable device
  2026-04-04  0:19 [PATCH net] net: avoid nul-deref trying to bind mp to incapable device Jakub Kicinski
@ 2026-04-04 22:43 ` Daniel Borkmann
  2026-04-06 23:55 ` Mina Almasry
  1 sibling, 0 replies; 3+ messages in thread
From: Daniel Borkmann @ 2026-04-04 22:43 UTC (permalink / raw)
  To: Jakub Kicinski, davem
  Cc: netdev, edumazet, pabeni, andrew+netdev, horms, sdf, almasrymina

On 4/4/26 2:19 AM, Jakub Kicinski wrote:
> Sashiko points out that we use qops in __net_mp_open_rxq()
> but never validate they are null. This was introduced when
> check was moved from netdev_rx_queue_restart().
> 
> Look at ops directly instead of the locking config.
> qops imply netdev_need_ops_lock(). We used netdev_need_ops_lock()
> initially to signify that the real_num_rx_queues check below
> is safe without rtnl_lock, but I'm not sure if this is actually
> clear to most people, anyway.
> 
> Fixes: da7772a2b4ad ("net: move mp->rx_page_size validation to __net_mp_open_rxq()")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: avoid nul-deref trying to bind mp to incapable device
  2026-04-04  0:19 [PATCH net] net: avoid nul-deref trying to bind mp to incapable device Jakub Kicinski
  2026-04-04 22:43 ` Daniel Borkmann
@ 2026-04-06 23:55 ` Mina Almasry
  1 sibling, 0 replies; 3+ messages in thread
From: Mina Almasry @ 2026-04-06 23:55 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, sdf,
	daniel

On Fri, Apr 3, 2026 at 5:19 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> Sashiko points out that we use qops in __net_mp_open_rxq()
> but never validate they are null. This was introduced when
> check was moved from netdev_rx_queue_restart().
>
> Look at ops directly instead of the locking config.
> qops imply netdev_need_ops_lock(). We used netdev_need_ops_lock()
> initially to signify that the real_num_rx_queues check below
> is safe without rtnl_lock, but I'm not sure if this is actually
> clear to most people, anyway.
>
> Fixes: da7772a2b4ad ("net: move mp->rx_page_size validation to __net_mp_open_rxq()")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Mina Almasry <almasrymina@google.com>

Honestly better for readability too. I was confused why we check that
we need a lock here but never netdev_lock_ops AFAICT.

FWIW I'm not sure why this branch doesn't have NL_SET_ERR_MSG, but I
didn't look deeply. Probably doesn't matter.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-04-06 23:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-04  0:19 [PATCH net] net: avoid nul-deref trying to bind mp to incapable device Jakub Kicinski
2026-04-04 22:43 ` Daniel Borkmann
2026-04-06 23:55 ` Mina Almasry

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox