linux-unionfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Chengguang Xu <cgxu519@mykernel.net>
Cc: Miklos Szeredi <miklos@szeredi.hu>,
	overlayfs <linux-unionfs@vger.kernel.org>
Subject: Re: [PATCH] ovl: fix error for ovl_fill_super()
Date: Mon, 1 Mar 2021 09:57:29 +0200	[thread overview]
Message-ID: <CAOQ4uxhijbRwH8BxzZ6CMqZiJB_cK6k_QWFB-sg4zH=H-n3+0w@mail.gmail.com> (raw)
In-Reply-To: <20210301061930.3459022-1-cgxu519@mykernel.net>

On Mon, Mar 1, 2021 at 8:28 AM Chengguang Xu <cgxu519@mykernel.net> wrote:
>
> There are some places should return -EINVAL instead of
> -ENOMEM in ovl_fill_super(), so just fix it.
>
> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
> ---
>  fs/overlayfs/super.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index fdd72f1a9c5e..3dda1d530a43 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -1984,6 +1984,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
>         if (numlower > OVL_MAX_STACK) {
>                 pr_err("too many lower directories, limit is %d\n",
>                        OVL_MAX_STACK);
> +               err = -EINVAL;
>                 goto out_err;
>         }
>
> @@ -2010,6 +2011,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
>         /* alloc/destroy_inode needed for setting up traps in inode cache */
>         sb->s_op = &ovl_super_operations;
>
> +       err = -EINVAL;
>         if (ofs->config.upperdir) {
>                 struct super_block *upper_sb;
>

OK. But we are not really being consistent in the ways we set err in this
function, which means we will have more of these bugs in the future
(and we have had them in the past as well...)

So either set err = -EINVAL after every pr_err() in this function:
- "missing lowerdir"
- "too many lower dirs"
- "missing workdir"

Or always set err before the tests including err = -ENOMEM
before allocating layers.

Mixing seems worse than either choice IMO.

Maybe Miklos has a better idea for tyding this up?

Thanks,
Amir.

  reply	other threads:[~2021-03-01  7:58 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-01  6:19 [PATCH] ovl: fix error for ovl_fill_super() Chengguang Xu
2021-03-01  7:57 ` Amir Goldstein [this message]
2021-03-24 16:12   ` Miklos Szeredi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAOQ4uxhijbRwH8BxzZ6CMqZiJB_cK6k_QWFB-sg4zH=H-n3+0w@mail.gmail.com' \
    --to=amir73il@gmail.com \
    --cc=cgxu519@mykernel.net \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).