linux-kernel - Re: [PATCH 1/2] shmem: fix tmpfs to handle the huge= option properly

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:   Thu, 22 Sep 2016 19:33:34 -0700 (PDT)
From:   Hugh Dickins <hughd@...gle.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
cc:     "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
        Toshi Kani <toshi.kani@....com>,
        Hillf Danton <hillf.zj@...baba-inc.com>,
        dan.j.williams@...el.com, mawilcox@...rosoft.com, hughd@...gle.com,
        kirill.shutemov@...ux.intel.com, linux-nvdimm@...ts.01.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] shmem: fix tmpfs to handle the huge= option
 properly

On Thu, 22 Sep 2016, Aneesh Kumar K.V wrote:
> Toshi Kani <toshi.kani@....com> writes:
> 
> > shmem_get_unmapped_area() checks SHMEM_SB(sb)->huge incorrectly,
> > which leads to a reversed effect of "huge=" mount option.
> >
> > Fix the check in shmem_get_unmapped_area().
> >
> > Note, the default value of SHMEM_SB(sb)->huge remains as
> > SHMEM_HUGE_NEVER.  User will need to specify "huge=" option to
> > enable huge page mappings.
> >
> 
> Any update on getting this merged ?
> 
> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@...ux.vnet.ibm.com>

Yikes, how did we ever not notice this?  Very embarrassing.

Huge thank you to Hillf for spotting it (only now do I rediscover
your June mail: I'm sorry, my attention has been fully elsewhere).

Big thank you to Toshi for sending the patch,
and to Aneesh for now sounding the alarm.

The only reassurance is that at least all the rest of it has
been under test for the last few months, via the SHMEM_HUGE_FORCE
override.  So it's not as if none of the code has been tested,
but I am still mystified why it hasn't been obvious without.

To the patch,
Acked-by: Hugh Dickins <hughd@...gle.com>
but I wish I could dream up a more emphatic tag.

Andrew, please please grab this and send it in!

Thank you,
Hugh

> 
> > Reported-by: Hillf Danton <hillf.zj@...baba-inc.com>
> > Signed-off-by: Toshi Kani <toshi.kani@....com>
> > Cc: Andrew Morton <akpm@...ux-foundation.org>
> > Cc: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> > Cc: Hugh Dickins <hughd@...gle.com>
> > ---
> >  mm/shmem.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mm/shmem.c b/mm/shmem.c
> > index fd8b2b5..aec5b49 100644
> > --- a/mm/shmem.c
> > +++ b/mm/shmem.c
> > @@ -1980,7 +1980,7 @@ unsigned long shmem_get_unmapped_area(struct file *file,
> >  				return addr;
> >  			sb = shm_mnt->mnt_sb;
> >  		}
> > -		if (SHMEM_SB(sb)->huge != SHMEM_HUGE_NEVER)
> > +		if (SHMEM_SB(sb)->huge == SHMEM_HUGE_NEVER)
> >  			return addr;
> >  	}
> >  
> >
> > --