Re: [PATCH] hugetlbfs: fix error handling in init_hugetlbfs_fs()

[Mike Kravetz]

On 10/28/19 9:36 PM, Chengguang Xu wrote:
>  ---- å ææä, 2019-10-29 05:27:01 Mike Kravetz <mike.kravetz@xxxxxxxxxx> æå ----
>  > 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@xxxxxxxxxxxx>
>  > Fixes: 32021982a324 ("hugetlbfs: Convert to fs_context")
>  > Cc: stable@xxxxxxxxxxxxxxx
>  > Signed-off-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
>  > ---
>  >  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.

Yes, even the original code did not clean up properly in the case of
all mount errors.  This will explicitly mount the default_hstate_idx
hstate first.  Then, optionally mount other hstates.  It will make the
error handling and cleanup more explicit.