linux-kernel - Re: [PATCH 0/1] Dead stores in maple-tree

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]

Message-ID: <20221027171604.cdu43dzcnsu7wy2i@revolver>
Date:   Thu, 27 Oct 2022 17:16:13 +0000
From:   Liam Howlett <liam.howlett@...cle.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>
CC:     Lukas Bulwahn <lukas.bulwahn@...il.com>,
        Matthew Wilcox <willy@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "maple-tree@...ts.infradead.org" <maple-tree@...ts.infradead.org>
Subject: Re: [PATCH 0/1] Dead stores in maple-tree

* Dan Carpenter <dan.carpenter@...cle.com> [221027 03:43]:
> On Wed, Oct 26, 2022 at 02:23:19PM +0000, Liam Howlett wrote:
> > * Lukas Bulwahn <lukas.bulwahn@...il.com> [221026 08:01]:
> > > Dear maple-tree authors, dear Liam, dear Matthew,
> > > 
> > > there are some Dead Stores that clang-analyzer reports:
> > > 
> > > lib/maple_tree.c:2906:2: warning: Value stored to 'last' is never read [clang-analyzer-deadcode.DeadStores]
> > > lib/maple_tree.c:2907:2: warning: Value stored to 'prev_min' is never read [clang-analyzer-deadcode.DeadStores]
> > > 
> > > I addressed these two cases, which were most obvious and clear to fix;
> > > see patch of this one-element series.
> > > 
> > > Further, clang-analyzer reports more, which I did not address:
> > > 
> > > lib/maple_tree.c:332:2: warning: Value stored to 'node' is never read [clang-analyzer-deadcode.DeadStores]
> > > lib/maple_tree.c:337:2: warning: Value stored to 'node' is never read [clang-analyzer-deadcode.DeadStores]
> > > 
> > > Unclear to me if the tool is wrong or right in its analysis here for the two functions above.
> > 
> > The tool is correct but these aren't going anywhere.  They are compiled
> > out and are needed for the future.
> > 
> 
> lib/maple_tree.c

~line 302:
/* Bit 1 indicates the root is a node */
#define MAPLE_ROOT_NODE                 0x02
/* maple_type stored bit 3-6 */
#define MAPLE_ENODE_TYPE_SHIFT          0x03
/* Bit 2 means a NULL somewhere below */
#define MAPLE_ENODE_NULL                0x04


>    330  static inline void mte_set_full(const struct maple_enode *node)
>    331  {
>    332          node = (void *)((unsigned long)node & ~MAPLE_ENODE_NULL);
>    333  }
>    334  
>    335  static inline void mte_clear_full(const struct maple_enode *node)
>    336  {
>    337          node = (void *)((unsigned long)node | MAPLE_ENODE_NULL);
>    338  }

Looking at the code.... the analysis is correct and these need to be
fixed.  Thanks Dan & Lukas.

> 
> That code is really puzzling...  How far into the future before it starts
> making sense?

If you want to know details like this, you can look at the comments in
the header and c file - that's where the development information
resides.  Information about a node is encoded in the last bits of that
nodes pointer - since they are aligned we can use a mask to restore the
pointer.  Internally I refer to nodes with encoded information as
maple_enodes.  This part is to do with finding out if there is a free
index within the range the node holds.  Think about searching for the
next available index for a unique identifier.

Thanks,
Liam