Re: bug in may_dedupe_file allows to deduplicate files we aren't allowed to write to

[Hugo Valtier]

> My guess is that not many users try to dedupe other users' files,
> so this feature was never used and nobody complained.

+1

Thx for the answer, I'm new to this to be sure I understood what you meant:
> You should add an xfstest for this and include a
> _fixed_by_kernel_commit and that will signal all the distros that
> care to backport the fix.

So right now I wait for 6.9 to be released soon enough then
I then submit my patch which invert the condition.
Once that is merged in some tree (fsdevel I guess ?) I submit a patch for
xfstest which adds a regression test and has _fixed_by_kernel_commit
mentioning the commit just merged in the fsdevel linux tree.

Le sam. 4 mai 2024 à 11:43, Amir Goldstein <amir73il@xxxxxxxxx> a écrit :
>
> On Sat, May 4, 2024 at 7:49 AM Hugo Valtier <hugo@xxxxxxxxxx> wrote:
> >
> > For context I am making a file based deduplication tool.
> >
> > I found that in this commit
> > 5de4480ae7f8 ("vfs: allow dedupe of user owned read-only files")
> > it states:
> > > - the process could get write access
> >
> > However the behavior added in allow_file_dedupe now may_dedupe_file is opposite:
> > > +       if (!inode_permission(file_inode(file), MAY_WRITE))
> > > +               return true
> >
> > I've tested that I can create an other readonly file as root and have
> > my unprivileged user deduplicate it however if I then make the file
> > other writeable I cannot anymore*.
> > It doesn't make sense to me why giving write permissions on a file
> > should remove the permission to deduplicate*.
>
> True. Here is the discussion about adding "could have been opened w"
> to allow dedupe:
> https://lore.kernel.org/linux-fsdevel/20180517230150.GA28045@xxxxxxxxxxxxx/
>
> >
> > I'm not sure on how to fix this, flipping the condition would work but
> > that is a breaking change and idk if this is ok here.
> > Adding a check to also users who have write access to the file would
> > remove all the logic here since you would always be allowed to dedup
> > FDs you managed to get your hands on.
> >
> > Any input on this welcome, thx
>
> My guess is that not many users try to dedupe other users' files,
> so this feature was never used and nobody complained.
> What use case do you think flipping the condition could break?
> breaking uapi is not about theoretical use cases and in any
> case this needs to be marked with Fixes: and can be backported
> as far as anyone who cares wants to backport.
>
> You should add an xfstest for this and include a
> _fixed_by_kernel_commit and that will signal all the distros that
> care to backport the fix.
>
> Thanks,
> Amir.