From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752635AbcB2NoD (ORCPT ); Mon, 29 Feb 2016 08:44:03 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51319 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750772AbcB2NoA (ORCPT ); Mon, 29 Feb 2016 08:44:00 -0500 Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 3798903 From: David Howells In-Reply-To: <20160229130924.GV17997@ZenIV.linux.org.uk> References: <20160229130924.GV17997@ZenIV.linux.org.uk> <20160225163927.GW17997@ZenIV.linux.org.uk> <20160226212137.GF17997@ZenIV.linux.org.uk> <20160226220759.GH17997@ZenIV.linux.org.uk> <20160227222748.GI17997@ZenIV.linux.org.uk> <20160228170133.GM17997@ZenIV.linux.org.uk> <20160228200100.GP17997@ZenIV.linux.org.uk> To: Al Viro Cc: dhowells@redhat.com, Dmitry Vyukov , Ian Kent , =?iso-8859-1?Q?Micka=EBl_Sala=FCn?= , "linux-fsdevel@vger.kernel.org" , LKML , syzkaller , Kostya Serebryany , Alexander Potapenko , Sasha Levin , Linus Torvalds Subject: Re: fs: NULL deref in atime_needs_update MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <14803.1456753436.1@warthog.procyon.org.uk> Date: Mon, 29 Feb 2016 13:43:56 +0000 Message-ID: <14804.1456753436@warthog.procyon.org.uk> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Al Viro wrote: > David, Linus, do you see any problems with that? To me it looks saner > that way and as cheap as the current code, but I might be missing something > here... You're effectively converting to this: read d_seq.sequence smp_rmb() read d_inode, d_flags smp_rmb() check d_seq.sequence in the read path and this: write d_seq.sequence smp_wmb() write d_inode, d_flags smp_wmb() write d_seq.sequence This should work - especially if we're wangling these sequence points anyway, and so have to pay the barrier penalties whatever. In fact, you actually take a barrier out, I think. I have had a problem with getting the ordering of d_inode and d_flags right because of __d_clear_type_and_inode() where we're required to unset a dentry so that it can be repurposed[*] as a negative dentry rather than replacing it. This is something we for performance sake - and it's something we can only do if the dentry isn't referenced, whereas if we properly followed the RCU model, we would have to wait a grace period after delisting an unlinked dentry before we could repurpose it - but that significantly slows down rename, unlink and rmdir. [*] Al and I disagree on whether this is a reuse or merely a change of state. David