Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230521082813.17025-1-andrea.righi@canonical.com>
In-Reply-To: <20230521082813.17025-1-andrea.righi@canonical.com>
From:   Amir Goldstein <amir73il@gmail.com>
Date:   Sat, 12 Aug 2023 19:28:38 +0300
Message-ID: <CAOQ4uxhE8Y0eLDoKu9tfxvjJo-7K3aAmt4jN9+Q37Jzk7LxokA@mail.gmail.com>
Subject: Re: [PATCH v3 0/3] overlayfs: debugging check for valid superblock
To:     Andrea Righi <andrea.righi@canonical.com>
Cc:     Miklos Szeredi <miklos@szeredi.hu>, linux-unionfs@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Sun, May 21, 2023 at 11:28=E2=80=AFAM Andrea Righi
<andrea.righi@canonical.com> wrote:
>
> OVL_FS() is used to get a struct ovl_fs from a sturct super_block, but
> we don't have any check to determine if the superblock is valid or not.
>
> This can lead to unexpected behaviors or bugs that are pretty hard to
> track down.
>
> Add an explicit WARN_ON_ONCE() check to OVL_FS() to make sure it's
> always used with a valid overlayfs superblock.
>
> To avoid enabling this additional pendatic check everywhere, introduce
> the new config option CONFIG_OVERLAY_FS_DEBUG, that can be used in the
> future also for other additional debugging checks.
>
> Maybe a nicer solution could be to return an error from OVL_FS() when
> it's used with an invalid superblock and propagate the error in the rest
> of overlayfs code, but for now having at least the possibility to
> trigger a warning can help to catch potential bugs in advance.
>

Applied to overlayfs-next with following changes:
- open code WARN_ON_ONCE() in OVL_FS() (Miklos)
- rebase and add missing OVL_FS() accessors

Thanks,
Amir.

> Changelog (v2 -> v3):
>  - do not hide is_ovl_fs_sb() under CONFIG_OVERLAY_FS_DEBUG
>  - split consistent use of OVL_FS() and superblock validation in two
>    separate patches
>
> Changelog (v1 -> v2):
>  - replace BUG_ON() with WARN_ON_ONCE()
>  - introduce CONFIG_OVERLAY_FS_DEBUG
>
> Andrea Righi (3):
>       ovl: Kconfig: introduce CONFIG_OVERLAY_FS_DEBUG
>       ovl: make consistent use of OVL_FS()
>       ovl: validate superblock in OVL_FS()
>
>  fs/overlayfs/Kconfig     |  9 +++++++++
>  fs/overlayfs/copy_up.c   |  2 +-
>  fs/overlayfs/export.c    | 10 +++++-----
>  fs/overlayfs/inode.c     |  8 ++++----
>  fs/overlayfs/namei.c     |  2 +-
>  fs/overlayfs/ovl_entry.h | 14 ++++++++++++++
>  fs/overlayfs/super.c     | 12 ++++++------
>  fs/overlayfs/util.c      | 18 +++++++++---------
>  8 files changed, 49 insertions(+), 26 deletions(-)
>