* tmpfs: fixups to use of the new mount API
@ 2019-08-19 22:09 Hugh Dickins
2019-08-19 22:13 ` Andrew Morton
0 siblings, 1 reply; 3+ messages in thread
From: Hugh Dickins @ 2019-08-19 22:09 UTC (permalink / raw)
To: David Howells
Cc: Al Viro, Andrew Morton, linux-fsdevel, linux-kernel, linux-mm
Several fixups to shmem_parse_param() and tmpfs use of new mount API:
mm/shmem.c manages filesystem named "tmpfs": revert "shmem" to "tmpfs"
in its mount error messages.
/sys/kernel/mm/transparent_hugepage/shmem_enabled has valid options
"deny" and "force", but they are not valid as tmpfs "huge" options.
The "size" param is an alternative to "nr_blocks", and needs to be
recognized as changing max_blocks. And where there's ambiguity, it's
better to mention "size" than "nr_blocks" in messages, since "size" is
the variant shown in /proc/mounts.
shmem_apply_options() left ctx->mpol as the new mpol, so then it was
freed in shmem_free_fc(), and the filesystem went on to use-after-free.
shmem_parse_param() issue "tmpfs: Bad value for '%s'" messages just
like fs_parse() would, instead of a different wording. Where config
disables "mpol" or "huge", say "tmpfs: Unsupported parameter '%s'".
Signed-off-by: Hugh Dickins <hughd@google.com>
---
mm/shmem.c | 80 ++++++++++++++++++++++++++-------------------------
1 file changed, 42 insertions(+), 38 deletions(-)
--- mmotm/mm/shmem.c 2019-08-17 11:33:16.557900238 -0700
+++ linux/mm/shmem.c 2019-08-19 13:37:29.184001050 -0700
@@ -3432,13 +3432,11 @@ static const struct fs_parameter_enum sh
{ Opt_huge, "always", SHMEM_HUGE_ALWAYS },
{ Opt_huge, "within_size", SHMEM_HUGE_WITHIN_SIZE },
{ Opt_huge, "advise", SHMEM_HUGE_ADVISE },
- { Opt_huge, "deny", SHMEM_HUGE_DENY },
- { Opt_huge, "force", SHMEM_HUGE_FORCE },
{}
};
const struct fs_parameter_description shmem_fs_parameters = {
- .name = "shmem",
+ .name = "tmpfs",
.specs = shmem_param_specs,
.enums = shmem_param_enums,
};
@@ -3448,9 +3446,9 @@ static void shmem_apply_options(struct s
unsigned long inodes_in_use)
{
struct shmem_fs_context *ctx = fc->fs_private;
- struct mempolicy *old = NULL;
- if (test_bit(Opt_nr_blocks, &ctx->changes))
+ if (test_bit(Opt_nr_blocks, &ctx->changes) ||
+ test_bit(Opt_size, &ctx->changes))
sbinfo->max_blocks = ctx->max_blocks;
if (test_bit(Opt_nr_inodes, &ctx->changes)) {
sbinfo->max_inodes = ctx->max_inodes;
@@ -3459,8 +3457,11 @@ static void shmem_apply_options(struct s
if (test_bit(Opt_huge, &ctx->changes))
sbinfo->huge = ctx->huge;
if (test_bit(Opt_mpol, &ctx->changes)) {
- old = sbinfo->mpol;
- sbinfo->mpol = ctx->mpol;
+ /*
+ * Update sbinfo->mpol now while stat_lock is held.
+ * Leave shmem_free_fc() to free the old mpol if any.
+ */
+ swap(sbinfo->mpol, ctx->mpol);
}
if (fc->purpose != FS_CONTEXT_FOR_RECONFIGURE) {
@@ -3471,8 +3472,6 @@ static void shmem_apply_options(struct s
if (test_bit(Opt_mode, &ctx->changes))
sbinfo->mode = ctx->mode;
}
-
- mpol_put(old);
}
static int shmem_parse_param(struct fs_context *fc, struct fs_parameter *param)
@@ -3498,7 +3497,7 @@ static int shmem_parse_param(struct fs_c
rest++;
}
if (*rest)
- return invalf(fc, "shmem: Invalid size");
+ goto bad_value;
ctx->max_blocks = DIV_ROUND_UP(size, PAGE_SIZE);
break;
@@ -3506,55 +3505,59 @@ static int shmem_parse_param(struct fs_c
rest = param->string;
ctx->max_blocks = memparse(param->string, &rest);
if (*rest)
- return invalf(fc, "shmem: Invalid nr_blocks");
+ goto bad_value;
break;
+
case Opt_nr_inodes:
rest = param->string;
ctx->max_inodes = memparse(param->string, &rest);
if (*rest)
- return invalf(fc, "shmem: Invalid nr_inodes");
+ goto bad_value;
break;
+
case Opt_mode:
ctx->mode = result.uint_32 & 07777;
break;
+
case Opt_uid:
ctx->uid = make_kuid(current_user_ns(), result.uint_32);
if (!uid_valid(ctx->uid))
- return invalf(fc, "shmem: Invalid uid");
+ goto bad_value;
break;
case Opt_gid:
ctx->gid = make_kgid(current_user_ns(), result.uint_32);
if (!gid_valid(ctx->gid))
- return invalf(fc, "shmem: Invalid gid");
+ goto bad_value;
break;
case Opt_huge:
-#ifdef CONFIG_TRANSPARENT_HUGE_PAGECACHE
- if (!has_transparent_hugepage() &&
- result.uint_32 != SHMEM_HUGE_NEVER)
- return invalf(fc, "shmem: Huge pages disabled");
-
ctx->huge = result.uint_32;
+ if (ctx->huge != SHMEM_HUGE_NEVER &&
+ !(IS_ENABLED(CONFIG_TRANSPARENT_HUGE_PAGECACHE) &&
+ has_transparent_hugepage()))
+ goto unsupported_parameter;
break;
-#else
- return invalf(fc, "shmem: huge= option disabled");
-#endif
-
- case Opt_mpol: {
-#ifdef CONFIG_NUMA
- struct mempolicy *mpol;
- if (mpol_parse_str(param->string, &mpol))
- return invalf(fc, "shmem: Invalid mpol=");
- mpol_put(ctx->mpol);
- ctx->mpol = mpol;
-#endif
- break;
- }
+
+ case Opt_mpol:
+ if (IS_ENABLED(CONFIG_NUMA)) {
+ struct mempolicy *mpol;
+ if (mpol_parse_str(param->string, &mpol))
+ goto bad_value;
+ mpol_put(ctx->mpol);
+ ctx->mpol = mpol;
+ break;
+ }
+ goto unsupported_parameter;
}
__set_bit(opt, &ctx->changes);
return 0;
+
+unsupported_parameter:
+ return invalf(fc, "tmpfs: Unsupported parameter '%s'", param->key);
+bad_value:
+ return invalf(fc, "tmpfs: Bad value for '%s'", param->key);
}
/*
@@ -3572,14 +3575,15 @@ static int shmem_reconfigure(struct fs_c
unsigned long inodes_in_use;
spin_lock(&sbinfo->stat_lock);
- if (test_bit(Opt_nr_blocks, &ctx->changes)) {
+ if (test_bit(Opt_nr_blocks, &ctx->changes) ||
+ test_bit(Opt_size, &ctx->changes)) {
if (ctx->max_blocks && !sbinfo->max_blocks) {
spin_unlock(&sbinfo->stat_lock);
- return invalf(fc, "shmem: Can't retroactively limit nr_blocks");
+ return invalf(fc, "tmpfs: Cannot retroactively limit size");
}
if (percpu_counter_compare(&sbinfo->used_blocks, ctx->max_blocks) > 0) {
spin_unlock(&sbinfo->stat_lock);
- return invalf(fc, "shmem: Too few blocks for current use");
+ return invalf(fc, "tmpfs: Too small a size for current use");
}
}
@@ -3587,11 +3591,11 @@ static int shmem_reconfigure(struct fs_c
if (test_bit(Opt_nr_inodes, &ctx->changes)) {
if (ctx->max_inodes && !sbinfo->max_inodes) {
spin_unlock(&sbinfo->stat_lock);
- return invalf(fc, "shmem: Can't retroactively limit nr_inodes");
+ return invalf(fc, "tmpfs: Cannot retroactively limit inodes");
}
if (ctx->max_inodes < inodes_in_use) {
spin_unlock(&sbinfo->stat_lock);
- return invalf(fc, "shmem: Too few inodes for current use");
+ return invalf(fc, "tmpfs: Too few inodes for current use");
}
}
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: tmpfs: fixups to use of the new mount API
2019-08-19 22:09 tmpfs: fixups to use of the new mount API Hugh Dickins
@ 2019-08-19 22:13 ` Andrew Morton
2019-08-19 22:21 ` Hugh Dickins
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2019-08-19 22:13 UTC (permalink / raw)
To: Hugh Dickins
Cc: David Howells, Al Viro, linux-fsdevel, linux-kernel, linux-mm
On Mon, 19 Aug 2019 15:09:14 -0700 (PDT) Hugh Dickins <hughd@google.com> wrote:
> Several fixups to shmem_parse_param() and tmpfs use of new mount API:
>
> mm/shmem.c manages filesystem named "tmpfs": revert "shmem" to "tmpfs"
> in its mount error messages.
>
> /sys/kernel/mm/transparent_hugepage/shmem_enabled has valid options
> "deny" and "force", but they are not valid as tmpfs "huge" options.
>
> The "size" param is an alternative to "nr_blocks", and needs to be
> recognized as changing max_blocks. And where there's ambiguity, it's
> better to mention "size" than "nr_blocks" in messages, since "size" is
> the variant shown in /proc/mounts.
>
> shmem_apply_options() left ctx->mpol as the new mpol, so then it was
> freed in shmem_free_fc(), and the filesystem went on to use-after-free.
>
> shmem_parse_param() issue "tmpfs: Bad value for '%s'" messages just
> like fs_parse() would, instead of a different wording. Where config
> disables "mpol" or "huge", say "tmpfs: Unsupported parameter '%s'".
Is this
Fixes: 144df3b288c41 ("vfs: Convert ramfs, shmem, tmpfs, devtmpfs, rootfs to use the new mount API")?
and a Cc:stable is appropriate?
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: tmpfs: fixups to use of the new mount API
2019-08-19 22:13 ` Andrew Morton
@ 2019-08-19 22:21 ` Hugh Dickins
0 siblings, 0 replies; 3+ messages in thread
From: Hugh Dickins @ 2019-08-19 22:21 UTC (permalink / raw)
To: Andrew Morton
Cc: Hugh Dickins, David Howells, Al Viro, linux-fsdevel,
linux-kernel, linux-mm
On Mon, 19 Aug 2019, Andrew Morton wrote:
> On Mon, 19 Aug 2019 15:09:14 -0700 (PDT) Hugh Dickins <hughd@google.com> wrote:
>
> > Several fixups to shmem_parse_param() and tmpfs use of new mount API:
> >
> > mm/shmem.c manages filesystem named "tmpfs": revert "shmem" to "tmpfs"
> > in its mount error messages.
> >
> > /sys/kernel/mm/transparent_hugepage/shmem_enabled has valid options
> > "deny" and "force", but they are not valid as tmpfs "huge" options.
> >
> > The "size" param is an alternative to "nr_blocks", and needs to be
> > recognized as changing max_blocks. And where there's ambiguity, it's
> > better to mention "size" than "nr_blocks" in messages, since "size" is
> > the variant shown in /proc/mounts.
> >
> > shmem_apply_options() left ctx->mpol as the new mpol, so then it was
> > freed in shmem_free_fc(), and the filesystem went on to use-after-free.
> >
> > shmem_parse_param() issue "tmpfs: Bad value for '%s'" messages just
> > like fs_parse() would, instead of a different wording. Where config
> > disables "mpol" or "huge", say "tmpfs: Unsupported parameter '%s'".
>
> Is this
>
> Fixes: 144df3b288c41 ("vfs: Convert ramfs, shmem, tmpfs, devtmpfs, rootfs to use the new mount API")?
That's the patch and the SHA1 I saw when I looked it up in linux-next
yesterday: I don't know if the SHA1 will change before it reaches Linus.
>
> and a Cc:stable is appropriate?
No: this is just a fix for linux-next and mmotm at present.
Hugh
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-08-19 22:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-19 22:09 tmpfs: fixups to use of the new mount API Hugh Dickins
2019-08-19 22:13 ` Andrew Morton
2019-08-19 22:21 ` Hugh Dickins
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).