linux-kernel - Re: [patch 1/3] flex_array: fix get function for elements in base starting at non-zero

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: <alpine.DEB.2.00.0908171738030.1766@chino.kir.corp.google.com>
Date:	Mon, 17 Aug 2009 17:49:32 -0700 (PDT)
From:	David Rientjes <rientjes@...gle.com>
To:	Dave Hansen <dave@...ux.vnet.ibm.com>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [patch 1/3] flex_array: fix get function for elements in base
 starting at non-zero

On Mon, 17 Aug 2009, Dave Hansen wrote:

> On Mon, 2009-08-17 at 16:46 -0700, David Rientjes wrote:
> > This fixes the bug by only checking for NULL parts when all elements do
> > not fit in the base structure when flex_array_get() is used.  Otherwise,
> > fa_element_to_part_nr() will always be 0 since there are no parts
> > structures needed and such element may never have been put.  Thus, it
> > will remain NULL due to the kzalloc() of the base.
> 
> Whew.  That one took me way longer to grok than it should have.  Thanks
> for finding this.  Just to be clear, there is only a bug in
> flex_array_get(), right?  The flex_array_put() change is completely
> separate and is intended to optimize the case where we know the pointer
> can't be NULL.
> 
> This definitely fixes a bug, but do you mind if we do it a bit
> differently?  The compiler should be able to take care of figuring out
> when that pointer actually needs to be checked, and I think it looks a
> bit nicer as it stands.   
> 

Your patch doesn't optimize the check away when all the elements are 
stored in the base structure, gcc doesn't infer that part must be valid 
based upon previous dereferences.  In fact, the resulting assembly would 
probably show the calculation of the element offset from `part' to happen 
in all cases iff part is non-NULL.

The flex_array_put() optimization is done for the same reason.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/