On Thu, Jul 4, 2024 at 10:05 AM Zhihao Cheng <chengzhihao1@xxxxxxxxxx> wrote:
>
> The max count of lowerdir is OVL_MAX_STACK[500], which is broken by
> commit 37f32f526438("ovl: fix memory leak in ovl_parse_param()") for
> parameter Opt_lowerdir. Since commit 819829f0319a("ovl: refactor layer
> parsing helpers") and commit 24e16e385f22("ovl: add support for
> appending lowerdirs one by one") added check ovl_mount_dir_check() in
> function ovl_parse_param_lowerdir(), the 'ctx->nr' should be smaller
> than OVL_MAX_STACK, after commit 37f32f526438("ovl: fix memory leak in
> ovl_parse_param()") is applied, the 'ctx->nr' is updated before the
> check ovl_mount_dir_check(), which leads the max count of lowerdir
> to become 499 for parameter Opt_lowerdir.
> Fix it by replacing lower layers parsing code with the existing helper
> function ovl_parse_layer().
>
> Fixes: 37f32f526438 ("ovl: fix memory leak in ovl_parse_param()")
> Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>
> Signed-off-by: Zhihao Cheng <chengzhihao1@xxxxxxxxxx>
> ---
Looks good.
You may add
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
> fs/overlayfs/params.c | 40 +++++++---------------------------------
> 1 file changed, 7 insertions(+), 33 deletions(-)
>
> diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
> index 52e3860973b7..8dd834c7f291 100644
> --- a/fs/overlayfs/params.c
> +++ b/fs/overlayfs/params.c
> @@ -353,6 +353,8 @@ static void ovl_add_layer(struct fs_context *fc, enum ovl_opt layer,
> case Opt_datadir_add:
> ctx->nr_data++;
> fallthrough;
> + case Opt_lowerdir:
> + fallthrough;
> case Opt_lowerdir_add:
> WARN_ON(ctx->nr >= ctx->capacity);
> l = &ctx->lower[ctx->nr++];
> @@ -375,7 +377,7 @@ static int ovl_parse_layer(struct fs_context *fc, const char *layer_name, enum o
> if (!name)
> return -ENOMEM;
>
> - if (upper)
> + if (upper || layer == Opt_lowerdir)
> err = ovl_mount_dir(name, &path);
> else
> err = ovl_mount_dir_noesc(name, &path);
> @@ -431,7 +433,6 @@ static int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc)
> {
> int err;
> struct ovl_fs_context *ctx = fc->fs_private;
> - struct ovl_fs_context_layer *l;
> char *dup = NULL, *iter;
> ssize_t nr_lower, nr;
> bool data_layer = false;
> @@ -471,35 +472,11 @@ static int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc)
> goto out_err;
> }
>
> - if (nr_lower > ctx->capacity) {
> - err = -ENOMEM;
> - l = krealloc_array(ctx->lower, nr_lower, sizeof(*ctx->lower),
> - GFP_KERNEL_ACCOUNT);
> - if (!l)
> - goto out_err;
> -
> - ctx->lower = l;
> - ctx->capacity = nr_lower;
> - }
> -
> iter = dup;
> - l = ctx->lower;
> - for (nr = 0; nr < nr_lower; nr++, l++) {
> - ctx->nr++;
> - memset(l, 0, sizeof(*l));
> -
> - err = ovl_mount_dir(iter, &l->path);
> + for (nr = 0; nr < nr_lower; nr++) {
> + err = ovl_parse_layer(fc, iter, Opt_lowerdir);
> if (err)
> - goto out_put;
> -
> - err = ovl_mount_dir_check(fc, &l->path, Opt_lowerdir, iter, false);
> - if (err)
> - goto out_put;
> -
> - err = -ENOMEM;
> - l->name = kstrdup(iter, GFP_KERNEL_ACCOUNT);
> - if (!l->name)
> - goto out_put;
> + goto out_err;
>
> if (data_layer)
> ctx->nr_data++;
> @@ -517,7 +494,7 @@ static int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc)
> */
> if (ctx->nr_data > 0) {
> pr_err("regular lower layers cannot follow data lower layers");
> - goto out_put;
> + goto out_err;
> }
>
> data_layer = false;
> @@ -531,9 +508,6 @@ static int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc)
> kfree(dup);
> return 0;
>
> -out_put:
> - ovl_reset_lowerdirs(ctx);
> -
> out_err:
> kfree(dup);
>
> --
> 2.39.2
>
