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: <CAGudoHErv6sX+Tq=NNLL3b61Q70TeZxi93Nx_MEcMrQSg47JGA@mail.gmail.com>
Date: Sat, 5 Apr 2025 06:54:28 +0200
From: Mateusz Guzik <mjguzik@...il.com>
To: Christian Brauner <brauner@...nel.org>
Cc: Christoph Hellwig <hch@...radead.org>, Penglei Jiang <superman.xpt@...il.com>, viro@...iv.linux.org.uk, 
	jack@...e.cz, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	syzbot+5d8e79d323a13aa0b248@...kaller.appspotmail.com
Subject: Re: [PATCH] anon_inode: use a proper mode internally

On Fri, Apr 4, 2025 at 12:39 PM Christian Brauner <brauner@...nel.org> wrote:
>
> This allows the VFS to not trip over anonymous inodes and we can add
> asserts based on the mode into the vfs. When we report it to userspace
> we can simply hide the mode to avoid regressions. I've audited all
> direct callers of alloc_anon_inode() and only secretmen overrides i_mode
> and i_op inode operations but it already uses a regular file.
>
[snip]
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 6393d7c49ee6..0ad3336f5b49 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -1647,7 +1647,7 @@ struct inode *alloc_anon_inode(struct super_block *s)
>          * that it already _is_ on the dirty list.
>          */
>         inode->i_state = I_DIRTY;
> -       inode->i_mode = S_IRUSR | S_IWUSR;
> +       inode->i_mode = S_IFREG | S_IRUSR | S_IWUSR;
>         inode->i_uid = current_fsuid();
>         inode->i_gid = current_fsgid();
>         inode->i_flags |= S_PRIVATE;

Switching the mode to S_IFREG imo can open a can of worms (perhaps a
dedicated in-kernel only mode would be better? S_IFANON?), but that's
a long and handwave-y subject, so I'm going to stop at this remark.

I think the key here is to provide an invariant that anon inodes don't
pass MAY_EXEC in may_open() regardless of their specific i_mode and
which the kernel fails to provide at the moment afaics.

With the patch as proposed in the other thread this is handled with
MAY_EXEC check added to the default: clause.

With your patch this is going to depend on the i_mode not allowing
exec or at least the anon inode having a mount or sb with noexec flags
(thus failing path_noexec() check), but from code reading I'm not at
all confident it has one (if it does not, it should get it). Merely
depending on mode is imo too risky as sooner or later there may be a
way to add +x to it. That is to say I think your patch works, provided
the backing mount or sb for anon inodes is guaranteed to have noexec
on it.




--
Mateusz Guzik <mjguzik gmail.com>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ