From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Subject: Re: [PATCH] mnt: Fix a memory stomp in umount Date: Fri, 19 Dec 2014 00:03:42 +0000 Message-ID: <20141219000342.GS22149@ZenIV.linux.org.uk> References: <87d27gkia8.fsf@x220.int.ebiederm.org> <87h9wsiwwl.fsf@x220.int.ebiederm.org> <20141218210553.GQ22149@ZenIV.linux.org.uk> <87d27ghd1i.fsf@x220.int.ebiederm.org> <20141219000215.GR22149@ZenIV.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Linus Torvalds , linux-fsdevel To: "Eric W. Biederman" Return-path: Received: from zeniv.linux.org.uk ([195.92.253.2]:56980 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751198AbaLSADn (ORCPT ); Thu, 18 Dec 2014 19:03:43 -0500 Content-Disposition: inline In-Reply-To: <20141219000215.GR22149@ZenIV.linux.org.uk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, Dec 19, 2014 at 12:02:15AM +0000, Al Viro wrote: > On Thu, Dec 18, 2014 at 03:18:49PM -0600, Eric W. Biederman wrote: > > > We might have wanted to because before your change to an hlist that is > > what the code did. > > > > Having read through the code of propagate_next it does look like it > > iterates through the entire propagation hierarchy so there shouldn't be > > a need to visit mounts that we have placed on the propagation list. > > > > Any thoughts on using mnt_list instead of mnt_hash to allow the use of > > list_splice and list_move? > > Frankly, I don't see much benefit in that. What's wrong with actually > adding > __hlist_splice(struct hlist_node *head, > struct hlist_node *tail, > struct hlist_node **where) > { > next = *where; > *where = head; > head->pprev = where; > if (next) { > tail->next = next; > next->pprev = tail; > } > } > > and use it as > __hlist_splice(tmp_list.first, &last->mnt_hash, &unmounted.first); > (and I'd start with if (unlikely(!mnt)) return; /* idiot caller */) > > Another primitive would be something like > hlist_transplant(struct hlist_head **from, struct hlist_head **to) hlist_transplant(struct hlist_head **to, struct hlist_head **from), that is... > { > *to = *from; > if (!hlist_empty(*to)) { > to->first->pprev = to; > INIT_HLIST_HEAD(from); > } > } > > with namespace_unlock() starting with > hlist_transplant(&head, &unmounted); > if (likely(hlist_empty(&head))) { > up_write(&namespace_sem); > return; > } > /* undo decrements we'd done in umount_tree() */ > ... > > Linus, would that work for you, or would you prefer something more fancy? > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html