linux-kernel - Re: [PATCH AUTOSEL 5.6 14/68] driver core: Reevaluate dev->links.need_for

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: <CAGETcx8RBjr6rzR7=m6LuA=OQOT2Hh4ioPndUP8mkpYLve+6yw@mail.gmail.com>
Date:   Fri, 10 Apr 2020 09:25:51 -0700
From:   Saravana Kannan <saravanak@...gle.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Sasha Levin <sashal@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        stable <stable@...r.kernel.org>
Subject: Re: [PATCH AUTOSEL 5.6 14/68] driver core: Reevaluate
 dev->links.need_for_probe as suppliers are added

On Thu, Apr 9, 2020 at 11:52 PM Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
>
> On Thu, Apr 09, 2020 at 11:39:55PM -0700, Saravana Kannan wrote:
> > On Thu, Apr 9, 2020 at 11:29 PM Greg Kroah-Hartman
> > <gregkh@...uxfoundation.org> wrote:
> > >
> > > On Thu, Apr 09, 2020 at 11:45:39PM -0400, Sasha Levin wrote:
> > > > From: Saravana Kannan <saravanak@...gle.com>
> > > >
> > > > [ Upstream commit 1745d299af5b373abad08fa29bff0d31dc6aff21 ]
> > > >
> > > > A previous patch 03324507e66c ("driver core: Allow
> > > > fwnode_operations.add_links to differentiate errors") forgot to update
> > > > all call sites to fwnode_operations.add_links. This patch fixes that.
> > > >
> > > > Legend:
> > > > -> Denotes RHS is an optional/potential supplier for LHS
> > > > => Denotes RHS is a mandatory supplier for LHS
> > > >
> > > > Example:
> > > >
> > > > Device A => Device X
> > > > Device A -> Device Y
> > > >
> > > > Before this patch:
> > > > 1. Device A is added.
> > > > 2. Device A is marked as waiting for mandatory suppliers
> > > > 3. Device X is added
> > > > 4. Device A is left marked as waiting for mandatory suppliers
> > > >
> > > > Step 4 is wrong since all mandatory suppliers of Device A have been
> > > > added.
> > > >
> > > > After this patch:
> > > > 1. Device A is added.
> > > > 2. Device A is marked as waiting for mandatory suppliers
> > > > 3. Device X is added
> > > > 4. Device A is no longer considered as waiting for mandatory suppliers
> > > >
> > > > This is the correct behavior.
> > > >
> > > > Fixes: 03324507e66c ("driver core: Allow fwnode_operations.add_links to differentiate errors")
> > > > Signed-off-by: Saravana Kannan <saravanak@...gle.com>
> > > > Link: https://lore.kernel.org/r/20200222014038.180923-2-saravanak@google.com
> > > > Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > > > Signed-off-by: Sasha Levin <sashal@...nel.org>
> > > > ---
> > > >  drivers/base/core.c | 8 ++++++--
> > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > > > index dbb0f9130f42d..d32a3aefff32f 100644
> > > > --- a/drivers/base/core.c
> > > > +++ b/drivers/base/core.c
> > > > @@ -523,9 +523,13 @@ static void device_link_add_missing_supplier_links(void)
> > > >
> > > >       mutex_lock(&wfs_lock);
> > > >       list_for_each_entry_safe(dev, tmp, &wait_for_suppliers,
> > > > -                              links.needs_suppliers)
> > > > -             if (!fwnode_call_int_op(dev->fwnode, add_links, dev))
> > > > +                              links.needs_suppliers) {
> > > > +             int ret = fwnode_call_int_op(dev->fwnode, add_links, dev);
> > > > +             if (!ret)
> > > >                       list_del_init(&dev->links.needs_suppliers);
> > > > +             else if (ret != -ENODEV)
> > > > +                     dev->links.need_for_probe = false;
> > > > +     }
> > > >       mutex_unlock(&wfs_lock);
> > > >  }
> > >
> > > For some reason this wasn't for stable kernels, but I can't remember.
> >
> > It *is* for stable kernels too. It is an actual bug that's fixable in
> > stable kernels. I think this might have been the one patch that I
> > bundled into an unrelated series, but called it out as an unrelated
> > bug. Maybe my wording in that email threw you off?
>
> I think it did, sorry.  So no problem adding this to the stable trees so
> I can go add it right now?

Yes, please do.

-Saravana