linux-kernel - Re: [PATCH 0/3] io path options + reflink (mild security implications)

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] [day] [month] [year] [list]

Message-ID: <20241112105349.kg347ogngynglr4t@quack3>
Date: Tue, 12 Nov 2024 11:53:49 +0100
From: Jan Kara <jack@...e.cz>
To: Kent Overstreet <kent.overstreet@...ux.dev>
Cc: linux-bcachefs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, brauner@...nel.org,
	sforshee@...nel.org, viro@...iv.linux.org.uk, jack@...e.cz
Subject: Re: [PATCH 0/3] io path options + reflink (mild security
 implications)

On Mon 11-11-24 22:35:32, Kent Overstreet wrote:
> so, I've been fleshing out various things at the intersection of io path
> options + rebalance + reflink, and this is the last little bit
> 
> background: bcachefs has io path options that can be set filesystem
> wide, or per inode, and when changed rebalance automatically picks them
> up and does the right thing
> 
> reflink adds a wrinkle, which is that we'd like e.g. recursively setting
> the foreground/background targets on some files to move them to the
> appropriate device (or nr_replicas etc.), like other data - but if a
> user did a reflink copy of some other user's data and then set
> nr_replicas=1, that would be bad.
> 
> so this series adds a flag to reflink pointers for "may propagate option
> changes", which can then be set at remap_file_range() time based on
> vfs level permission checks.
> 
> so, question for everyone: is write access to the source file what we
> want? or should it be stricter, i.e. ownership matches?

Well, if I understand the impact properly, this seems similar to the
effects vfs_fileattr_set() can have on a file and there we have:

        if (!inode_owner_or_capable(idmap, inode))
                return -EPERM;

So I'd say ownership match would be more consistent.

> then, we're currently missing mnt_idmap plumbing to remap_file_range()
> to do said permissions checks - do we want to do that? or is there an
> easier way?

Well, if you have struct file, you have the mount and thus idmap through
file_mnt_idmap(file). And struct file is available in remap_file_range()
call stack so I'm not sure what is the problem exactly.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR