linux-kernel - Re: [PATCH RESEND 1/1] vfs: Really check for inode ptr in lookup

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]

Message-ID: <20191102061706.GA10268@ZenIV.linux.org.uk>
Date:   Sat, 2 Nov 2019 06:17:06 +0000
From:   Al Viro <viro@...iv.linux.org.uk>
To:     "Paul E. McKenney" <paulmck@...nel.org>
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        wugyuan@...ibm.com, jlayton@...nel.org, hsiangkao@....com,
        Jan Kara <jack@...e.cz>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Ritesh Harjani <riteshh@...ux.ibm.com>
Subject: Re: [PATCH RESEND 1/1] vfs: Really check for inode ptr in lookup_fast

On Fri, Nov 01, 2019 at 11:46:22PM +0000, Al Viro wrote:
> on anything except alpha that would be pretty much automatic and
> on alpha we get the things along the lines of
> 
> 	f = fdt[n]
> 	mb
> 	d = f->f_path.dentry
> 	i = d->d_inode
> 	assert(i != NULL)
> vs.
> 	see that d->d_inode is non-NULL
> 	f->f_path.dentry = d
> 	mb
> 	fdt[n] = f
> 
> IOW, the barriers that make it safe to fetch the fields of struct file
> (rcu_dereference_raw() in __fcheck_files() vs. smp_store_release()
> in __fd_install() in the above) should *hopefully* take care of all
> stores visible by the time of do_dentry_open().  Sure, alpha cache
> coherency is insane, but AFAICS it's not _that_ insane.
> 
> Question to folks familiar with alpha memory model:
> 
> A = 0, B = NULL, C = NULL
> CPU1:
> 	A = 1
> 
> CPU2:
> 	r1 = A
> 	if (r1) {
> 		B = &A
> 		mb
> 		C = &B
> 	}
> 
> CPU3:
> 	r2 = C;
> 	mb
> 	if (r2) {	// &B
> 		r3 = *r2	// &A
> 		r4 = *r3	// 1
> 		assert(r4 == 1)
> 	}
> 
> is the above safe on alpha?

Hmm...  After digging through alpha manuals, it should be -

U1: W A, 1

V1: R A, 1
V2: W B, &A
V3: MB
V4: W C, &B

W1: R C, &B
W2: MB
W3: R B, &A
W4: R A, 0

is rejected since
	U1 BEFORE V1 (storage and visibility)
	V1 BEFORE V3 BEFORE V4 (processor issue order constraints)
	V4 BEFORE W1 (storage and visibility)
	W1 BEFORE W2 BEFORE W4 (processor issue order constraints)
and W4 BEFORE U1 (storage and visibility), which is impossible
due to BEFORE being acyclic and transitive.

I might very well be missing something, though...  Paul, could you
take a look and tell if the above makes sense?