linux-kernel - Re: [PATCH 0/7 v3] overlay filesystem prototype

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:	Wed, 22 Sep 2010 11:50:30 +0200
From:	Miklos Szeredi <miklos@...redi.hu>
To:	Neil Brown <neilb@...e.de>
CC:	miklos@...redi.hu, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, vaurora@...hat.com,
	viro@...iv.linux.org.uk
Subject: Re: [PATCH 0/7 v3] overlay filesystem prototype

On Tue, 21 Sep 2010, Neil Brown wrote:
> On Mon, 20 Sep 2010 20:04:04 +0200
> Miklos Szeredi <miklos@...redi.hu> wrote:
> 
> > Here's an updated patch series.
> > 
> > For now I reverted Neil's revalidation patch.  Not requiring strict
> > read-only would make sense for just trying it out and experimenting.
> > But for real uses, I'm not sure...
> 
> :-)
> 
> I think you significantly reduce the value by insisting on read-only but as
> this is purely a theoretical perspective at the moment (I have no concrete
> use-case) I won't push it.

I'm not insisting on read-only.  I think there's value to enabling
external modification in special circumstances, but not by default.

I also think it might not be a good idea to waste CPU cycles and brain
cycles on implementing (an always imperfect) revalidation.  A similar
mechanism is "mount -oremount mountpoint", which simply throws out all
unused dentries, effectively forcing a revalidation of the whole
filesystem.  Less selective than ->d_revalidate(), but it shouldn't be
a big issue since the underlying filesystem dentries remain cached.

> I had another patch I was working on which caused overlayfs to keep negative
> dentries in upperdentry or lowerdentry rather than just setting them to
> NULL.  This would allow revalidation to notice objects appearing in the
> underlying filesystem.  I guess you won't want that now .... I think it made
> some of the code a bit neater, but I never finished it so I cannot be sure of
> the overall effect.
> 
> I'm curious as to why upperdentry is now called __upperdentry - it isn't
> clear from a quick reading..

(I need to add more comments to the code...)

The underscores are meant to imply that unlinke ->lowerdentry it's not
qute safe to access directly.  See discussion about memory barriers:

  http://lkml.org/lkml/2010/9/19/142

It's not as clear as one might like.  To sum up:

  - an smp_read_barrier_depends() is needed on SMP Alpha, but not on
    other archs

  - an ACCESS_ONCE() is needed on some theoretical compiler that does
    weird optimizations, but apparently not on current versions of gcc

The VFS lives happily without either, locklessly accessing
dentry->d_inode.

Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/