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: <87ldxnrkxw.fsf@intel.com>
Date: Wed, 13 Nov 2024 11:30:03 -0800
From: Vinicius Costa Gomes <vinicius.gomes@...el.com>
To: Amir Goldstein <amir73il@...il.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

Amir Goldstein <amir73il@...il.com> writes:

> On Thu, Nov 7, 2024 at 1:57 AM Vinicius Costa Gomes
> <vinicius.gomes@...el.com> wrote:

[...]

>
> 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...

Hm, if the leak wasn't there before and we didn't touch anything related to
prepare_creds(), I think that points to the leak being real.

But I see your point, still not seeing it.

This code should be equivalent to the code we have now (just boot
tested):

----
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 136a2c7fb9e5..7ebc2fd3097a 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -576,8 +576,7 @@ static int ovl_setup_cred_for_create(struct dentry *dentry, struct inode *inode,
         * 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);
+       WARN_ON_ONCE(override_creds_light(override_cred) != ovl_creds(dentry->d_sb));

        return 0;
 }
----

Does it change anything? (I wouldn't think so, just to try something)

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


Cheers,
-- 
Vinicius