Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 29 Oct 2019 12:36:26 +0800
From:   Chengguang Xu <cgxu519@mykernel.net>
Reply-To: cgxu519@mykernel.net
To:     "Mike Kravetz" <mike.kravetz@oracle.com>
Cc:     "linux-mm" <linux-mm@kvack.org>,
        "linux-kernel" <linux-kernel@vger.kernel.org>,
        "David Howells" <dhowells@redhat.com>,
        "Andrew Morton" <akpm@linux-foundation.org>
Message-ID: <16e15cd0096.1068d5c9f40168.8315245997167313680@mykernel.net>
In-Reply-To: <ba6cd4a4-a1cd-82c0-5ea1-5e20112f8f6b@oracle.com>
References: <20191017103822.8610-1-cgxu519@mykernel.net> <ba6cd4a4-a1cd-82c0-5ea1-5e20112f8f6b@oracle.com>
Subject: Re: [PATCH] hugetlbfs: fix error handling in init_hugetlbfs_fs()
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: ZohoCN Mail
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

 ---- =E5=9C=A8 =E6=98=9F=E6=9C=9F=E4=BA=8C, 2019-10-29 05:27:01 Mike Krave=
tz <mike.kravetz@oracle.com> =E6=92=B0=E5=86=99 ----
 > 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().
 > >=20
 > > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
 >=20
 > 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?
 >=20
 > From 3144f0a9d18f1408e831fb3eb49a06636a11d902 Mon Sep 17 00:00:00 2001
 > From: Mike Kravetz <mike.kravetz@oracle.com>
 > Date: Mon, 28 Oct 2019 14:08:42 -0700
 > Subject: [PATCH] mm/hugetlbfs: fix error handling when setting up mounts
 >=20
 > 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.
 >=20
 > Reported-by: Chengguang Xu <cgxu519@mykernel.net>
 > Fixes: 32021982a324 ("hugetlbfs: Convert to fs_context")
 > Cc: stable@vger.kernel.org
 > Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
 > ---
 >  fs/hugetlbfs/inode.c | 10 ++++++----
 >  1 file changed, 6 insertions(+), 4 deletions(-)
 >=20
 > 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 =3D 0;
 >      for_each_hstate(h) {
 >          mnt =3D mount_one_hugetlbfs(h);
 > -        if (IS_ERR(mnt) && i =3D=3D 0) {
 > +        if (IS_ERR(mnt)) {
 > +            hugetlbfs_vfsmount[i] =3D NULL;
 >              error =3D PTR_ERR(mnt);
 > -            goto out;
 > +        } else {
 > +            hugetlbfs_vfsmount[i] =3D mnt;
 >          }
 > -        hugetlbfs_vfsmount[i] =3D mnt;
 >          i++;
 >      }
 > =20
 > -    return 0;
 > +    if (hugetlbfs_vfsmount[default_hstate_idx] !=3D NULL)
 > +        return 0;

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

Thanks,
Chengguang

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