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:   Sun, 3 Nov 2019 16:35:24 +0000
From:   Al Viro <viro@...iv.linux.org.uk>
To:     linux-fsdevel@...r.kernel.org
Cc:     Ritesh Harjani <riteshh@...ux.ibm.com>,
        linux-kernel@...r.kernel.org, wugyuan@...ibm.com,
        jlayton@...nel.org, hsiangkao@....com, Jan Kara <jack@...e.cz>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: [RFC] lookup_one_len_unlocked() lousy calling conventions

On Sat, Nov 02, 2019 at 06:08:42PM +0000, Al Viro wrote:

> It is converging to a reasonably small and understandable surface, actually,
> most of that being in core pathname resolution.  Two big piles of nightmares
> left to review - overlayfs and (somewhat surprisingly) setxattr call chains,
> the latter due to IMA/EVM/LSM insanity...

One thing found while digging through overlayfs (and responsible for several
remaining pieces from the assorted pile):

lookup_one_len_unlocked() calling conventions are wrong for its callers.
Namely, 11 out of 12 callers really want ERR_PTR(-ENOENT) on negatives.
Most of them take care to check, some rely upon that being impossible in
their case.  Interactions with dentry turning positive right after
lookup_one_len_unlocked() has returned it are of varying bugginess...

The only exception is ecryptfs, where we do lookup in the underlying fs
on ecryptfs_lookup() and want to retain a negative dentry if we get one.

Not sure what's the best way to handle that - it looks like we want
a primitive with different behaviour on negatives, so the most conservative
variant would be to add such, leaving lookup_one_len_unlocked() use
for ecryptfs (and dealing with barriers properly in there), with
everybody else switching to lookup_positive_unlocked(), or whatever
we call that new primitive.

Other variants:

1) add a primitve with existing lookup_one_len_unlocked() behaviour,
changing lookup_one_len_unlocked() itselt to new calling conventions.
Switch ecryptfs to that, drop now-useless checks from other callers.

2) get rid of pinning negative underlying dentries in ecryptfs.
The cost will be doing lookup in underlying layer twice on something
like mkdir(2), the second one quite likely served out of dcache.
That would make _all_ callers of lookup_one_len_unlocked() need
the same calling conventions.

Preferences?

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ