netdev - Re: [PATCH net] gve: Add adminq lock for creating and destroying multiple queues

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: <CAG-FcCPexBHubfzRcVUsBXRYkY0v-ricG-yqLd5q1j051cbEoQ@mail.gmail.com>
Date: Thu, 24 Apr 2025 11:37:07 -0700
From: Ziwei Xiao <ziweixiao@...gle.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Harshitha Ramamurthy <hramamurthy@...gle.com>, netdev@...r.kernel.org, jeroendb@...gle.com, 
	andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com, 
	pabeni@...hat.com, pkaligineedi@...gle.com, willemb@...gle.com, 
	shailend@...gle.com, linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH net] gve: Add adminq lock for creating and destroying
 multiple queues

On Wed, Apr 23, 2025 at 5:14 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Thu, 17 Apr 2025 20:43:23 +0000 Harshitha Ramamurthy wrote:
> > Also this patch cleans up the error handling code of
> > gve_adminq_destroy_tx_queue.
>
> >  static int gve_adminq_destroy_tx_queue(struct gve_priv *priv, u32 queue_index)
> >  {
> >       union gve_adminq_command cmd;
> > -     int err;
> >
> >       memset(&cmd, 0, sizeof(cmd));
> >       cmd.opcode = cpu_to_be32(GVE_ADMINQ_DESTROY_TX_QUEUE);
> > @@ -808,11 +820,7 @@ static int gve_adminq_destroy_tx_queue(struct gve_priv *priv, u32 queue_index)
> >               .queue_id = cpu_to_be32(queue_index),
> >       };
> >
> > -     err = gve_adminq_issue_cmd(priv, &cmd);
> > -     if (err)
> > -             return err;
> > -
> > -     return 0;
> > +     return gve_adminq_issue_cmd(priv, &cmd);
> >  }
>
> You mean this cleanup? That's not appropriate for a stable fix...
>
> Could you also explain which callers of this core are not already
> under rtnl_lock and/pr the netdev instance lock?
I discovered this and thought that this applied more widely, but upon
rereading it turns out it only applies to upcoming timestamping
patches and a previous flow steering code attempt that was scuttled.
Current callers are under rtnl_lock or netdev_lock. Should not have
been sent to the net. So will send as part of the timestamping series.
Thanks.


> --
> pw-bot: cr