linux-kernel - Re: [POC/RFC PATCH] overlayfs: constant inode numbers

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 29 Nov 2016 11:16:37 +0100
From:   Miklos Szeredi <miklos@...redi.hu>
To:     Amir Goldstein <amir73il@...il.com>
Cc:     "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: [POC/RFC PATCH] overlayfs: constant inode numbers

On Mon, Nov 28, 2016 at 12:56 PM, Amir Goldstein <amir73il@...il.com> wrote:
> On Mon, Nov 28, 2016 at 12:35 PM, Miklos Szeredi <miklos@...redi.hu> wrote:
>> On Mon, Nov 28, 2016 at 10:10 AM, Amir Goldstein <amir73il@...il.com> wrote:

[...]

>>> I may be way off here, but why do you need to lookup entry and get ino
>>> from xattr at all? Wouldn't it be easier to not add the entry to the list if
>>> it was copied up and rely on the fact that it will be added to list in iter
>>> of lower layer with original ino with no extra effort?
>>
>> What about renamed entries?
>
> Right. I completely missed out on the rename case..
>
>> What about opaque ones?
>
> That's exactly the point of OVL_XATTR_INO IFF !OVL_XATTR_OPAQUE
> If you have OVL_XATTR_INO means entry cannot be opaque, so it should
> be safe to skip it

So that means if OVL_XATTR_OPAQUE is set then we don't need to check
OVL_XATTR_INO and if OVL_XATTR_INO is set we don't need to check
OVL_XATTR_OPAQUE.   But that doesn't help optimize readdir, because we
really want to check neither.


>> I do hope we can optimize directory reading, because doing lookup and
>> getxattr for all entries is going to hurt...
>>
>
> Possibly silly question:
> Do you know if programs really rely of d_ino from getdents to be sane/non-zero?
> And what are the implications of overlayfs readdir not exporting the real d_ino?

I don't know of any precedent, so it's a big unknown.

>> We do keep ino stable across rename.  We don't keep ino stable across
>> copy-up.  That's what this patch is trying to address.
>>
>> You are saying that we should have redirects for non-dir and drop
>> OVL_XATTR_INO?  That's another option, but it doesn't look like it
>> would simplify things...

There *is* actually a case where both 'opaque' and 'ino' make sense:
when an empty merged dir is exchanged for an empty opaque one in
ovl_clear_empty().

> Well, not sure if you noticed my redirect_fh (rediect by file handle) work.
> If differs from redirect by path in 2 major ways:
> 1. Like OVL_XATTR_INO, redirect is set on copy up (but only for dirs)
> 2. Lookup is much simpler (and most likely faster) then full path lookup
>
> It would be trivial to set oe->ino of merged dir from lower most entry in
> ovl_lookup().

Okay.  In fact always using the handle looks like a better option
overall.  File handle should be unique for the lifetime of the
filesystem, while inode numbers may be reused.

Biggest drawback of the file handle based redirects is exactly that:
makes backing up the overlay basically impossible, since file handles
won't work after a backup + restore.  But as an optimization, in
addition to path based redirects it would work fine and provide a good
way to get the stable ino.

Thanks,
Miklos