linux-kernel - Re: [Letux-kernel] BUG: drivers/pinctrl/core: races in pinctrl

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]

Date:   Tue, 19 Jun 2018 00:15:23 -0700
From:   Tony Lindgren <tony@...mide.com>
To:     "H. Nikolaus Schaller" <hns@...delico.com>
Cc:     Christ van Willegen <cvwillegen@...il.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        kernel@...a-handheld.com,
        Discussions about the Letux Kernel 
        <letux-kernel@...nphoenux.org>
Subject: Re: [Letux-kernel] BUG: drivers/pinctrl/core: races in
 pinctrl_groups and deferred probing

* H. Nikolaus Schaller <hns@...delico.com> [180619 06:42]:
> Hi Tony,
> 
> > Am 19.06.2018 um 08:11 schrieb Tony Lindgren <tony@...mide.com>:
> > 
> > * H. Nikolaus Schaller <hns@...delico.com> [180619 04:54]:
> >>>> I had seen the call sequence
> >>>> 
> >>>> 
> >>>> 
> >>>> w/o any lock inside.
> > 
> > So the sequence above has mutex added around adding the pin
> > controller specific functions and groups by the patch series
> > I posted for both pcs_parse_bits_in_pinctrl_entry() and
> > pcs_parse_one_pinctrl_entry(). So I think the above should
> > be fixed now. But please confirm to make sure I'm not mistaken.
> 
> Ah, now I see.
> 
> My dump_stack() added to pinctrl_generic_add_group() reported
> 
> [    6.155487] Hardware name: Generic OMAP36xx (Flattened Device Tree)
> [    6.162048] [<c01106ec>] (unwind_backtrace) from [<c010c058>] (show_stack+0x10/0x14)
> [    6.170166] [<c010c058>] (show_stack) from [<c074bc28>] (dump_stack+0x7c/0x9c)
> [    6.177734] [<c074bc28>] (dump_stack) from [<c042bbec>] (pinctrl_generic_add_group+0x48/0x90)
> [    6.186614] [<c042bbec>] (pinctrl_generic_add_group) from [<c043203c>] (pcs_dt_node_to_map+0x4b0/0x81c)
> [    6.196441] [<c043203c>] (pcs_dt_node_to_map) from [<c042ffd4>] (pinctrl_dt_to_map+0x1ec/0x2b8)
> [    6.205535] [<c042ffd4>] (pinctrl_dt_to_map) from [<c042d028>] (create_pinctrl+0x58/0x2f8)
> [    6.214141] [<c042d028>] (create_pinctrl) from [<c042d388>] (devm_pinctrl_get+0x2c/0x6c)
> [    6.222625] [<c042d388>] (devm_pinctrl_get) from [<c04f2e9c>] (pinctrl_bind_pins+0x3c/0x138)
> [    6.231445] [<c04f2e9c>] (pinctrl_bind_pins) from [<c04d3eb8>] (driver_probe_device+0xe8/0x318)
> [    6.240509] [<c04d3eb8>] (driver_probe_device) from [<c04d4168>] (__driver_attach+0x80/0xa4)
> [    6.249328] [<c04d4168>] (__driver_attach) from [<c04d264c>] (bus_for_each_dev+0x58/0x7c)
> 
> Apparently I didn't notice that pcs_parse_*_pinctrl_entry() are called
> inside pcs_dt_node_to_map() and are part of the call sequence.
> 
> Hence your new mutex locks calls to pinctrl_generic_add_group() as
> needed.
> 
> Obviously, the compiler has optimized away the nested calls to static
> functions and I had no previous experience with how the whole pinctrl code
> works (learning by debugging :).
> 
> So it looks sane and no need for further locks.

OK thanks for checking it.

Regards,

Tony