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
| ||
|
Message-ID: <20171227142853.b5agfi2kzo25g5ot@ltop.local> Date: Wed, 27 Dec 2017 15:28:54 +0100 From: Luc Van Oostenryck <luc.vanoostenryck@...il.com> To: Matthew Wilcox <willy@...radead.org> Cc: Josh Triplett <josh@...htriplett.org>, Ross Zwisler <ross.zwisler@...ux.intel.com>, linux-kernel@...r.kernel.org, Dave Hansen <dave.hansen@...el.com>, linux-mm@...ck.org, Matthew Wilcox <mawilcox@...rosoft.com> Subject: Re: [PATCH 2/2] Introduce __cond_lock_err On Fri, Dec 22, 2017 at 04:31:12AM -0800, Matthew Wilcox wrote: > On Thu, Dec 21, 2017 at 08:21:20PM -0800, Josh Triplett wrote: > > While I've got you, I've been looking at some other sparse warnings from > this file. There are several caused by sparse being unable to handle > the following construct: > > if (foo) > x = NULL; > else { > x = bar; > __acquire(bar); > } > if (!x) > return -ENOMEM; > > Writing it as: > > if (foo) > return -ENOMEM; > else { > x = bar; > __acquire(bar); > } > > works just fine. ie this removes the warning: It must be noted that these two versions are not equivalent (in the first version, it also returns with -ENOMEM if bar is NULL/zero). It must be noted that sparse's goal regarding the context imbalance is to give the warning if some point in the code can be reached via two paths (or more) and the lock state (the context) is not identical in each of these paths. > > Is there any chance sparse's dataflow analysis will be improved in the > near future? A lot of functions in the kernel have this context imbalance, really a lot. For example, any function doing conditional locking is a problem here. Happily when these functions are inlined, sparse, thanks to its optimizations, can remove some paths and merge some others. So yes, by adding some smartness to sparse, some of the false warnings will be removed, however: 1) some __must_hold()/__acquires()/__releases() annotations are missing, making sparse's job impossible. 2) a lot of the 'false warnings' are not so false because there is indeed two possible paths with different lock state 3) it has its limits (at the end, giving the correct warning is equivalent to the halting problem). Now, to answer to your question, I'm not aware of any effort that would make a significant differences (it would need, IMO, code hoisting & value range propagation). -- Luc Van Oostenryck
Powered by blists - more mailing lists