On Tue, Jun 11, 2019 at 6:29 AM Darrick J. Wong <darrick.wong@xxxxxxxxxx> wrote:
>
> On Mon, Jun 10, 2019 at 10:51:08PM -0400, Theodore Ts'o wrote:
> > On Mon, Jun 10, 2019 at 06:16:12PM -0700, Darrick J. Wong wrote:
> > > On Mon, Jun 10, 2019 at 08:26:06PM +0300, Amir Goldstein wrote:
> > > > read(2) is allowed from a swapfile, so copy_file_range(2) should
> > > > be allowed as well.
> > > >
> > > > Reported-by: Theodore Ts'o <tytso@xxxxxxx>
> > > > Fixes: 96e6e8f4a68d ("vfs: add missing checks to copy_file_range")
> > > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > > > ---
> > > >
> > > > Darrick,
> > > >
> > > > This fixes the generic/554 issue reported by Ted.
> > >
> > > Frankly I think we should go the other way -- non-root doesn't get to
> > > copy from or read from swap files.
> >
> > The issue is that without this patch, *root* doesn't get to copy from
> > swap files. Non-root shouldn't have access via Unix permissions. We
>
> I'm not sure even root should have that privilege - it's a swap file,
> and until you swapoff, it's owned by the kernel and we shouldn't let
> backup programs copy your swapped out credit card numbers onto tape.
>
I am not a security expert and I do not want to be, but I believe it's
better to have a complete security model before plugging random
"security holes".
That said. I don't have a strong feeling about allowing copy_file_range
from swap file. If someone complains and they have a valid use case,
we can always relax it then.
Anyway, as you saw, I removed the test case from xfstest, leaving
the behavior (as far as the testsuite cares) undefined.
Thanks,
Amir.
