linux-kernel - Re: [PATCH v2] fs,security: Fix file_set

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: <CAGudoHEQA5hZf_mpB0byt24doF7Kwj7XO-uJ2-oKm25DXX4s3Q@mail.gmail.com>
Date: Wed, 14 Aug 2024 09:44:31 +0200
From: Mateusz Guzik <mjguzik@...il.com>
To: Paul Moore <paul@...l-moore.com>
Cc: Mickaël Salaün <mic@...ikod.net>, 
	Christian Brauner <brauner@...nel.org>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org, 
	selinux@...r.kernel.org, Jan Kara <jack@...e.cz>, 
	Tahera Fahimi <fahimitahera@...il.com>, Al Viro <viro@...iv.linux.org.uk>, 
	Casey Schaufler <casey@...aufler-ca.com>, James Morris <jmorris@...ei.org>, Jann Horn <jannh@...gle.com>, 
	Ondrej Mosnacek <omosnace@...hat.com>, "Serge E . Hallyn" <serge@...lyn.com>, 
	Stephen Smalley <stephen.smalley.work@...il.com>
Subject: Re: [PATCH v2] fs,security: Fix file_set_fowner LSM hook inconsistencies

On Wed, Aug 14, 2024 at 1:39 AM Paul Moore <paul@...l-moore.com> wrote:
>
> I don't see how where the cred reference live will have any impact,
> you still need to get and drop references which will have an impact.
> There will always be something.
>

The patch as posted here adds 2 atomics in the fast path and that
indeed is a problem, but it can be trivially avoided -- either use
get/put_cred_many or make it so that the same pointer means the ref is
held implicitly (after all the f_cred one is guaranteed to be there
for the entire file's lifetime).

Either way extra overhead does not have to be there (modulo one branch
on teardown to check for mismatched creds) and can be considered a
non-factor.

I have no basis to comment on the idea behind the patch.

I'll note however that the patch to move f_owner out of struct file
(and have *not* present by default) is likely to come through, it
already landed here:
https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/commit/?h=vfs.misc&id=0e8540d012189259261c75360d2725a2107761e7

I don't know if it has any bearing on viability of the patch posted here.

-- 
Mateusz Guzik <mjguzik gmail.com>