linux-kernel - Re: [PATCH rcu 04/20] srcu: Begin offloading srcu_struct fields to srcu

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 3 Apr 2023 21:16:27 -0400
From:   Joel Fernandes <joel@...lfernandes.org>
To:     paulmck@...nel.org
Cc:     rcu@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel-team@...a.com, rostedt@...dmis.org, hch@....de,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Michał Mirosław <mirq-linux@...e.qmqm.pl>,
        Dmitry Osipenko <dmitry.osipenko@...labora.com>,
        Sachin Sant <sachinp@...ux.ibm.com>,
        "Zhang, Qiang1" <qiang1.zhang@...el.com>
Subject: Re: [PATCH rcu 04/20] srcu: Begin offloading srcu_struct fields to srcu_update

On Mon, Apr 3, 2023 at 9:06 PM Paul E. McKenney <paulmck@...nel.org> wrote:
[..]
> > In other words, if init_srcu_struct_nodes() returns false, then passing along
> > the return value of init_srcu_struct_nodes() back to the caller of
> > init_srcu_struct_fields() depends on whether is_static = true or false. That
> > seems a bit wrong to me, init_srcu_struct_fields() should always return
> > -ENOMEM  when init_srcu_struct_nodes() fails to allocate memory IMHO, whether
> > is_static is true or not.
> >
> > Sorry if I missed something subtle, and if the code is correct to begin with.
> > Also I feel the return paths could be made better to also fix the above issue
> > I mentioned. How about the following diff on top of the series, would it
> > work?
>
> Your restructuring looks like an excellent step forward, but given the late
> date, it might be best to avoid being in a rush.
>
> I -could- make the following small patch:
>
>         if (READ_ONCE(ssp->srcu_sup->srcu_size_state) == SRCU_SIZE_SMALL && SRCU_SIZING_IS_INIT()) {
>                 if (!init_srcu_struct_nodes(ssp, GFP_ATOMIC)) {
>                         if (!ssp->srcu_sup->sda_is_static) {
>                                 free_percpu(ssp->sda);
>                                 ssp->sda = NULL;
>                                 kfree(ssp->srcu_sup);
>                         }
>                         return -ENOMEM;
>                 } else {
>                         WRITE_ONCE(ssp->srcu_sup->srcu_size_state, SRCU_SIZE_BIG);
>                 }
>         }
>
> Except that this is a pre-existing bug that as far as we know no one
> is hitting, so the risk doesn't seem to stack up.  After all, if you
> are hitting memory exhaustion at boot or on module load, this bug is
> probably the least of your problems.  Even this fix looks to be v6.5
> material to me.
>
> So would you be willing to send a patch like the above that fixes the
> bug, and another like you have below to get a better error-path
> structure?  No hurry, the end of this month is perfectly fine.

Yes, I am happy to send patches along these lines by the end of the
month. I will make a note to do so.

> And again, good catch!

Thanks!

- Joel