linux-kernel - Re: [PATCH v4 4/4] ovl: Optimize override/revert creds

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOQ4uxgHwmAa4K3ca7i1G2gFQ1WBge855R19hgEk7BNy+EBqfg@mail.gmail.com>
Date: Wed, 13 Nov 2024 15:26:59 +0100
From: Amir Goldstein <amir73il@...il.com>
To: Vinicius Costa Gomes <vinicius.gomes@...el.com>, brauner@...nel.org, miklos@...redi.hu
Cc: hu1.chen@...el.com, malini.bhandaru@...el.com, tim.c.chen@...el.com, 
	mikko.ylinen@...el.com, linux-unionfs@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 4/4] ovl: Optimize override/revert creds

On Thu, Nov 7, 2024 at 1:57 AM Vinicius Costa Gomes
<vinicius.gomes@...el.com> wrote:
>
> Use override_creds_light() in ovl_override_creds() and
> revert_creds_light() in ovl_revert_creds_light().
>
> The _light() functions do not change the 'usage' of the credentials in
> question, as they refer to the credentials associated with the
> mounter, which have a longer lifetime.
>
> In ovl_setup_cred_for_create(), do not need to modify the mounter
> credentials (returned by override_creds()) 'usage' counter. Add a
> warning to verify that we are indeed working with the mounter
> credentials (stored in the superblock). Failure in this assumption
> means that creds may leak.
>
> Suggested-by: Christian Brauner <brauner@...nel.org>
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@...el.com>
> ---
>  fs/overlayfs/dir.c  | 7 ++++++-
>  fs/overlayfs/util.c | 4 ++--
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 09db5eb19242..136a2c7fb9e5 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -571,7 +571,12 @@ static int ovl_setup_cred_for_create(struct dentry *dentry, struct inode *inode,
>                 put_cred(override_cred);
>                 return err;
>         }
> -       put_cred(override_creds(override_cred));
> +
> +       /*
> +        * We must be called with creator creds already, otherwise we risk
> +        * leaking creds.
> +        */
> +       WARN_ON_ONCE(override_creds(override_cred) != ovl_creds(dentry->d_sb));
>         put_cred(override_cred);
>
>         return 0;

Vinicius,

While testing fanotify with LTP tests (some are using overlayfs),
kmemleak consistently reports the problems below.

Can you see the bug, because I don't see it.
Maybe it is a false positive...

Christian, Miklos,

Can you see a problem?

Thanks,
Amir.


unreferenced object 0xffff888008ad8240 (size 192):
  comm "fanotify06", pid 1803, jiffies 4294890084
  hex dump (first 32 bytes):
    01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace (crc ee6a93ea):
    [<00000000ab4340a4>] __create_object+0x22/0x83
    [<0000000053dcaf3b>] kmem_cache_alloc_noprof+0x156/0x1e6
    [<00000000b4a08c1d>] prepare_creds+0x1d/0xf9
    [<00000000c55dfb6c>] ovl_setup_cred_for_create+0x27/0x93
    [<00000000f82af4ee>] ovl_create_or_link+0x73/0x1bd
    [<0000000040a439db>] ovl_create_object+0xda/0x11d
    [<00000000fbbadf17>] lookup_open.isra.0+0x3a0/0x3ff
    [<0000000007a2faf0>] open_last_lookups+0x160/0x223
    [<00000000e7d8243a>] path_openat+0x136/0x1b5
    [<0000000004e51585>] do_filp_open+0x57/0xb8
    [<0000000053871b92>] do_sys_openat2+0x6f/0xc0
    [<000000004d76b8b7>] do_sys_open+0x3f/0x60
    [<000000009b0be238>] do_syscall_64+0x96/0xf8
    [<000000006ff466ad>] entry_SYSCALL_64_after_hwframe+0x76/0x7e