linux-kernel - Re: [PATCH RESEND v5 1/8] reset: Fix devm bulk optional exclusive control getter

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: <20220627222014.3qlbakgtyp3noweo@mobilestation>
Date:   Tue, 28 Jun 2022 01:20:14 +0300
From:   Serge Semin <fancer.lancer@...il.com>
To:     Dmitry Osipenko <dmitry.osipenko@...labora.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>
Cc:     Serge Semin <Sergey.Semin@...kalelectronics.ru>,
        Michael Turquette <mturquette@...libre.com>,
        Dmitry Osipenko <digetx@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Pavel Parkhomenko <Pavel.Parkhomenko@...kalelectronics.ru>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        linux-clk@...r.kernel.org, linux-mips@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND v5 1/8] reset: Fix devm bulk optional exclusive
 control getter

On Mon, Jun 27, 2022 at 01:35:28AM +0300, Dmitry Osipenko wrote:
> On 6/24/22 17:18, Serge Semin wrote:
> > Most likely due to copy-paste mistake the device managed version of the
> > denoted reset control getter has been implemented with invalid semantic,
> > which can be immediately spotted by having "WARN_ON(shared && acquired)"
> > warning in the system log as soon as the method is called. Anyway let's
> > fix it by altering the boolean arguments passed to the
> > __devm_reset_control_bulk_get() method from
> > - shared = true, optional = false, acquired = true
> > to
> > + shared = false, optional = true, acquired = true
> > That's what they were supposed to be in the first place (see the non-devm
> > version of the same method: reset_control_bulk_get_optional_exclusive()).
> > 
> > Fixes: 48d71395896d ("reset: Add reset_control_bulk API")
> > Signed-off-by: Serge Semin <Sergey.Semin@...kalelectronics.ru>
> > 
> > ---
> > 
> > Changelog v4:
> > - This is a new patch added on v4 lap of the series.
> > ---
> >  include/linux/reset.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/reset.h b/include/linux/reset.h
> > index 8a21b5756c3e..514ddf003efc 100644
> > --- a/include/linux/reset.h
> > +++ b/include/linux/reset.h
> > @@ -731,7 +731,7 @@ static inline int __must_check
> >  devm_reset_control_bulk_get_optional_exclusive(struct device *dev, int num_rstcs,
> >  					       struct reset_control_bulk_data *rstcs)
> >  {
> > -	return __devm_reset_control_bulk_get(dev, num_rstcs, rstcs, true, false, true);
> > +	return __devm_reset_control_bulk_get(dev, num_rstcs, rstcs, false, true, true);
> >  }
> >  
> >  /**
> 

> Good catch,
> 
> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@...labora.com>

Thanks.

> 
> Although, this patch should be sent as standalone since it's unrelated
> to the rest of the clk patches.

Mainly you are right, but I've discovered the problem in the framework
of this patchset development. So it was more suitable for me to submit
the patch in the framework of it.

There is an implicit context too. Due to the patchset containing the
reset-control-related patches I has been trying to attract the
Philipp' attention for about four months now, but with no luck.
Having the kernel API fix-patch in the series was supposed to
implicitly raise the patchset valuability for him. I don't know what
else to do, but Stephen said he wanted to see what Philipp thought of
the reset-part of the series.

-Sergey

> 
> -- 
> Best regards,
> Dmitry