Re: [PATCH] btrfs: use mount idmap for defrag permission check

在 2026/8/14 01:11, Seth Forshee 写道:
> On Thu, Aug 13, 2026 at 04:04:05PM +0930, Qu Wenruo wrote:
>>
>>
>> 在 2026/8/13 13:11, Tao Cui 写道:
>>> From: Tao Cui <[email protected]>
>>>
>>> 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 <[email protected]>
>>
>> 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.
> 
Agreed, I'll resend it as a behavior change with no Fixes tag.
> 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?
> 
The check isn't a privilege gate. 616d374efa23 added it to test "whether
the file could have been opened rw", and the caller can already do that
on the idmapped mount -- open O_RDWR, write(), fallocate(). Only the
regular-file branch changes, and defrag only reorganizes the caller's
own extents; the S_IFDIR branch (whole-subvolume defrag) keeps its
capable(CAP_SYS_ADMIN).

Nothing is weakened: !capable(CAP_SYS_ADMIN) stays exactly as is.

may_dedupe_file() in fs/remap_range.c already gates FIDEDUPERANGE the
same way -- capable(CAP_SYS_ADMIN), then i_uid_into_vfsuid(
file_mnt_idmap(file)) / inode_permission(idmap, MAY_WRITE) -- and dedupe
is stronger, since it can rewrite one file's extents from another file's
contents. Idmapped users can already dedupe on btrfs.

Thanks,
Tao

> Thanks,
> Seth