mbox series

[v3,0/3] overlayfs: debugging check for valid superblock

Message ID 20230521082813.17025-1-andrea.righi@canonical.com
Headers
Series overlayfs: debugging check for valid superblock |

Message

Andrea Righi May 21, 2023, 8:28 a.m. UTC 
  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.

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(-)

Comments

Amir Goldstein Aug. 12, 2023, 4:28 p.m. UTC | #1 
On Sun, May 21, 2023 at 11:28 AM 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(-)
>