linux-kernel - Re: [PATCH] hugetlbfs: fix error handling in init_hugetlbfs

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <16e15cd0096.1068d5c9f40168.8315245997167313680@mykernel.net>
Date:   Tue, 29 Oct 2019 12:36:26 +0800
From:   Chengguang Xu <cgxu519@...ernel.net>
To:     "Mike Kravetz" <mike.kravetz@...cle.com>
Cc:     "linux-mm" <linux-mm@...ck.org>,
        "linux-kernel" <linux-kernel@...r.kernel.org>,
        "David Howells" <dhowells@...hat.com>,
        "Andrew Morton" <akpm@...ux-foundation.org>
Subject: Re: [PATCH] hugetlbfs: fix error handling in init_hugetlbfs_fs()

 ---- 在 星期二, 2019-10-29 05:27:01 Mike Kravetz <mike.kravetz@...cle.com> 撰写 ----
 > On 10/17/19 3:38 AM, Chengguang Xu wrote:
 > > In order to avoid using incorrect mnt, we should set
 > > mnt to NULL when we get error from mount_one_hugetlbfs().
 > > 
 > > Signed-off-by: Chengguang Xu <cgxu519@...ernel.net>
 > 
 > Thanks for noticing this issue.  As mentioned in a previous e-mail,
 > there are additional issues that need to be addressed.  This loop
 > needs to initialize entries in the hugetlbfs_vfsmount array for all
 > hstates.  How about this patch?
 > 
 > From 3144f0a9d18f1408e831fb3eb49a06636a11d902 Mon Sep 17 00:00:00 2001
 > From: Mike Kravetz <mike.kravetz@...cle.com>
 > Date: Mon, 28 Oct 2019 14:08:42 -0700
 > Subject: [PATCH] mm/hugetlbfs: fix error handling when setting up mounts
 > 
 > It is assumed that the hugetlbfs_vfsmount[] array will contain
 > either a valid vfsmount pointer or NULL for each hstate after
 > initialization.  Changes made while converting to use fs_context
 > broke this assumption.
 > 
 > Reported-by: Chengguang Xu <cgxu519@...ernel.net>
 > Fixes: 32021982a324 ("hugetlbfs: Convert to fs_context")
 > Cc: stable@...r.kernel.org
 > Signed-off-by: Mike Kravetz <mike.kravetz@...cle.com>
 > ---
 >  fs/hugetlbfs/inode.c | 10 ++++++----
 >  1 file changed, 6 insertions(+), 4 deletions(-)
 > 
 > diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
 > index a478df035651..178389209561 100644
 > --- a/fs/hugetlbfs/inode.c
 > +++ b/fs/hugetlbfs/inode.c
 > @@ -1470,15 +1470,17 @@ static int __init init_hugetlbfs_fs(void)
 >      i = 0;
 >      for_each_hstate(h) {
 >          mnt = mount_one_hugetlbfs(h);
 > -        if (IS_ERR(mnt) && i == 0) {
 > +        if (IS_ERR(mnt)) {
 > +            hugetlbfs_vfsmount[i] = NULL;
 >              error = PTR_ERR(mnt);
 > -            goto out;
 > +        } else {
 > +            hugetlbfs_vfsmount[i] = mnt;
 >          }
 > -        hugetlbfs_vfsmount[i] = mnt;
 >          i++;
 >      }
 >  
 > -    return 0;
 > +    if (hugetlbfs_vfsmount[default_hstate_idx] != NULL)
 > +        return 0;

Maybe we should umount other non-null entries and release
used inodes for safety in error case.

Thanks,
Chengguang

 >  
 >   out:
 >      kmem_cache_destroy(hugetlbfs_inode_cachep);
 > -- 
 > 2.20.1
 > 
 >