linux-kernel - Re: [PATCH v9 09/12] lib/vsprintf: Make use of fwnode API to obtain node names and separators

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: <20200113091751.d63u7jbyh6p2rj23@pathway.suse.cz>
Date:   Mon, 13 Jan 2020 10:17:51 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     Sakari Ailus <sakari.ailus@...ux.intel.com>,
        linux-kernel@...r.kernel.org, rafael@...nel.org,
        Rob Herring <robh@...nel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        linux-acpi@...r.kernel.org, devicetree@...r.kernel.org,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Joe Perches <joe@...ches.com>
Subject: Re: [PATCH v9 09/12] lib/vsprintf: Make use of fwnode API to obtain
 node names and separators

On Fri 2020-01-03 10:35:55, Guenter Roeck wrote:
> On Fri, Jan 03, 2020 at 03:42:53PM +0100, Petr Mladek wrote:
> > On Fri 2020-01-03 13:21:45, Sakari Ailus wrote:
> > > Hi Guenter,
> > > 
> > > On Thu, Jan 02, 2020 at 02:20:41PM -0800, Guenter Roeck wrote:
> > > > Hi,
> > > > 
> > > > On Thu, Oct 03, 2019 at 03:32:16PM +0300, Sakari Ailus wrote:
> > > > > Instead of implementing our own means of discovering parent nodes, node
> > > > > names or counting how many parents a node has, use the newly added
> > > > > functions in the fwnode API to obtain that information.
> > > > > 
> > > > > Signed-off-by: Sakari Ailus <sakari.ailus@...ux.intel.com>
> > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> > > > > Reviewed-by: Petr Mladek <pmladek@...e.com>
> > > > > ---
> > > > 
> > > > This patch results in a lockdep splat when running one of my qemu
> > > > emulations. See below for log and bisect results. A complete log
> > > > is available at
> > > > https://kerneltests.org/builders/qemu-arm-master/builds/1408/steps/qemubuildcommand/logs/stdio
> > > > 
> > > > Guenter
> > > 
> > > Thank you for reporting this.
> > > 
> > > I looked into the issue, and indeed I can conform the patch introduces this
> > > as it takes the devtree_lock for printing the name of the fwnode. There is
> > 
> > I guess that you meant "is not".
> > 
> > 
> > > however chance of a deadlock in practice as the code in mm/slub.c does not
> > > deal with fwnodes (in which case acquiring devtree_lock could be possible),
> > > maybe for other reasons as well. The patch however introduces an unpleasant
> > > source of such warnings.
> > 
> > I agree that it is a false positive. alloc/free is called in OF code
> > under devtree_lock. But OF code is not called from alloc/free (slub.c)
> > and it should not happen.
> > 
> 
> Assuming that memory allocation is indeed called from code holding
> devtree_lock: The problem, as I see it, is that the order of acquiring
> locks is different. In OF code, the order is
> 	devtree_lock
> 	(&n->list_lock)->rlock

Yes, this happens when alloc is called in OF code under devtree_lock.

> Elsewhere, in %pOF print sequences, it is
> 	(&n->list_lock)->rlock
> 	devtree_lock

I believe that this order does not exist in reality. lockep "just"
connected this the two locks via logbuf_lock. When printk() is
called in the allocator:

	(&n->list_lock)->rlock
	logbuf_lock

and when %pOF is used in printk():

	logbuf_lock
	devtree_lock

>From this two lockdep assumes that it might be possible to
use %pOF in printk() from allocator code:

	(&n->list_lock)->rlock
	logbuf_lock
	devtree_lock

But I believe that this does not make sense and never happen reality.

That said, I would still prefer when %pOF could be implemented
without the lock. It would make it usable anywhere without any
risk of deadlock.

Sakari, what is your opinion, please?

Best Regards,
Petr