linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Seth Forshee <sforshee@kernel.org>
To: Tao Cui <cui.tao@linux.dev>, Qu Wenruo <wqu@suse.com>
Cc: clm@fb.com, dsterba@suse.com, josef@toxicpanda.com,
	brauner@kernel.org, linux-btrfs@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	Tao Cui <cuitao@kylinos.cn>
Subject: Re: [PATCH] btrfs: use mount idmap for defrag permission check
Date: Thu, 13 Aug 2026 12:11:36 -0500	[thread overview]
Message-ID: <an36yH30W965eqZk@ubuntu-x1> (raw)
In-Reply-To: <bd66065f-bae5-482a-84d6-a78850c9cebe@suse.com>

On Thu, Aug 13, 2026 at 04:04:05PM +0930, Qu Wenruo wrote:
> 
> 
> 在 2026/8/13 13:11, Tao Cui 写道:
> > From: Tao Cui <cuitao@kylinos.cn>
> > 
> > btrfs_ioctl_defrag() checks MAY_WRITE with nop_mnt_idmap, which skips
> > the mount idmap.  On an idmapped mount the owner comparison then uses
> > the caller's fsuid against the raw on-disk uid, dropping the mapping.
> > Every other permission/owner check in btrfs ioctl uses
> > file_mnt_idmap(file) (e.g. :1152, :1310, :1946); this one missed it.
> > 
> > Switch to file_mnt_idmap(file).  It equals nop_mnt_idmap on a normal
> > mount, and the check stays behind !capable(CAP_SYS_ADMIN), so only
> > unprivileged callers on idmapped btrfs change.  The RO-fd note in the
> > comment above is about the file descriptor, not this inode check, and
> > is unaffected.
> > 
> > Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> 
> Fixes: 4609e1f18e19 ("fs: port ->permission() to pass mnt_idmap")

This is a misattribution. Using nop_mnt_idmap there keeps the behavior
the same as it was before the commit. Switching to the mount idmap opens
up BTRFS_IOC_DEFRAG* to idmapped users when they weren't before, which
is a separate policy decision that didn't belong in that change.

Saying that these ioctls should be allowed for idmapped users just
because others are isn't a good justification. Why are these ioctls safe
for idmapped users?  Do they actually require this capability?

Thanks,
Seth

  reply	other threads:[~2026-08-13 17:14 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-13  3:41 [PATCH] btrfs: use mount idmap for defrag permission check Tao Cui
2026-08-13  6:34 ` Qu Wenruo
2026-08-13 17:11   ` Seth Forshee [this message]
2026-08-14  3:29     ` Qu Wenruo
2026-08-14  5:15     ` Tao Cui
2026-08-14 15:25       ` Seth Forshee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=an36yH30W965eqZk@ubuntu-x1 \
    --to=sforshee@kernel.org \
    --cc=brauner@kernel.org \
    --cc=clm@fb.com \
    --cc=cui.tao@linux.dev \
    --cc=cuitao@kylinos.cn \
    --cc=dsterba@suse.com \
    --cc=josef@toxicpanda.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=wqu@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).