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.
>From 7291f1da0d494cb64f6d219943c59a02e6d4fca7 Mon Sep 17 00:00:00 2001
From: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
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.
While fixing the hugetlbfs_vfsmount issue, it was discovered that
init_hugetlbfs_fs never did correctly clean up when encountering
a vfs mount error.
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 | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index a478df035651..26e3906c18fe 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -1461,28 +1461,41 @@ static int __init init_hugetlbfs_fs(void)
sizeof(struct hugetlbfs_inode_info),
0, SLAB_ACCOUNT, init_once);
if (hugetlbfs_inode_cachep == NULL)
- goto out2;
+ goto out;
error = register_filesystem(&hugetlbfs_fs_type);
if (error)
- goto out;
+ goto out_free;
+ /* default hstate mount is required */
+ mnt = mount_one_hugetlbfs(&hstates[default_hstate_idx]);
+ if (IS_ERR(mnt)) {
+ error = PTR_ERR(mnt);
+ goto out_unreg;
+ }
+ hugetlbfs_vfsmount[default_hstate_idx] = mnt;
+
+ /* other hstates are optional */
i = 0;
for_each_hstate(h) {
+ if (i == default_hstate_idx)
+ continue;
+
mnt = mount_one_hugetlbfs(h);
- if (IS_ERR(mnt) && i == 0) {
- error = PTR_ERR(mnt);
- goto out;
- }
- hugetlbfs_vfsmount[i] = mnt;
+ if (IS_ERR(mnt))
+ hugetlbfs_vfsmount[i] = NULL;
+ else
+ hugetlbfs_vfsmount[i] = mnt;
i++;
}
return 0;
- out:
+ out_unreg:
+ (void)unregister_filesystem(&hugetlbfs_fs_type);
+ out_free:
kmem_cache_destroy(hugetlbfs_inode_cachep);
- out2:
+ out:
return error;
}
fs_initcall(init_hugetlbfs_fs)
--
2.20.1
