linux-kernel - Re: [PATCH 4/7] ovl: add infrastructure for intercepting file ops

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

Date:   Thu, 24 Nov 2016 15:12:57 +0200
From:   Amir Goldstein <amir73il@...il.com>
To:     Miklos Szeredi <miklos@...redi.hu>
Cc:     Miklos Szeredi <mszeredi@...hat.com>,
        "linux-unionfs@...r.kernel.org" <linux-unionfs@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/7] ovl: add infrastructure for intercepting file ops

On Thu, Nov 24, 2016 at 2:03 PM, Miklos Szeredi <miklos@...redi.hu> wrote:
> On Thu, Nov 24, 2016 at 12:52 PM, Amir Goldstein <amir73il@...il.com> wrote:
>> On Thu, Nov 24, 2016 at 12:55 PM, Miklos Szeredi <mszeredi@...hat.com> wrote:
>
>>> +               /*
>>> +                * These should be intercepted, but they are very unlikely to be
>>> +                * a problem in practice.  Leave them alone for now.
>>
>> It could also be handled in vfs helpers.
>> Since these ops all start with establishing that src and dest are on
>> the same sb,
>> then the cost of copy up of src is the cost of clone_file_range from
>> lower to upper,
>> so it is probably worth to copy up src and leave those fops alone.
>>
>>> +                */
>>> +               ofop->fops.copy_file_range = orig->copy_file_range;
>>> +               ofop->fops.clone_file_range = orig->clone_file_range;
>>> +               ofop->fops.dedupe_file_range = orig->dedupe_file_range;
>
> Not sure I understand.  Why should we copy up src?  Copy up is the
> problem not the solution.
>

Maybe the idea is ill conceived, but the reasoning is:
To avoid the corner case of cloning from a stale lower src,
call d_real() in vfs helpers to always copy up src before cloning from it
and pass the correct file onwards.

It would have been crazy if we suggested the same for read_iter(),
so why it may make sense for clone?
because once you got that far into the vfs_clone_range() helper,
with src from lower and dst from upper, it means they are on the same sb
that supports clone, so it is likely that copy up is going to use clone and
be relatively cheap.

Pretty twisted. I agree... and not sure if this logic holds for copy_range as
well. Anyway, that's what I meant.

Amir.