netdev - Re: [PATCH net-next] bnxt_en: fix kernel panic in queue api functions

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMArcTUNz7rABdgk2TMZAOSpHFr+fW8cu_X1jHa1H4j3MuUUBg@mail.gmail.com>
Date: Fri, 5 Jul 2024 00:20:20 +0900
From: Taehee Yoo <ap420073@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, pabeni@...hat.com, edumazet@...gle.com, 
	michael.chan@...adcom.com, netdev@...r.kernel.org, somnath.kotur@...adcom.com, 
	dw@...idwei.uk, horms@...nel.org
Subject: Re: [PATCH net-next] bnxt_en: fix kernel panic in queue api functions

On Thu, Jul 4, 2024 at 10:33 PM Jakub Kicinski <kuba@...nel.org> wrote:
>

Hi Jakub,
Thanks a lot for review!


> On Thu, 4 Jul 2024 07:41:53 +0000 Taehee Yoo wrote:
> > bnxt_queue_{mem_alloc,start,stop} access bp->rx_ring array and this is
> > initialized while an interface is being up.
> > The rings are initialized as a number of channels.
> >
> > The queue API functions access rx_ring without checking both null and
> > ring size.
> > So, if the queue API functions are called when interface status is down,
> > they access an uninitialized rx_ring array.
> > Also if the queue index parameter value is larger than a ring, it
> > would also access an uninitialized rx_ring.
>
> Shouldn't the core be checking against dev->real_num_rx_queues instead ?

Oh, I missed it.
I agree the core should check dev->real_num_rx_queues.
But the current devmem TCP code checks dev->num_rx_queues instead of
dev->real_num_rx_queues.
I tested the below change, and it works well.
So, I will comment on it in Mina's patch.

diff --git a/net/core/devmem.c b/net/core/devmem.c
index 7afaf17801ef..da27778c2421 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -146,7 +146,7 @@ int net_devmem_bind_dmabuf_to_queue(struct
net_device *dev, u32 rxq_idx,
u32 xa_idx;
int err;

- if (rxq_idx >= dev->num_rx_queues)
+ if (rxq_idx >= dev->real_num_rx_queues)
return -ERANGE;

rxq = __netif_get_rx_queue(dev, rxq_idx);


Thanks a lot!
Taehee Yoo