From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from zeniv.linux.org.uk ([195.92.253.2]:58280 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932215AbcA3ReP (ORCPT ); Sat, 30 Jan 2016 12:34:15 -0500 Date: Sat, 30 Jan 2016 17:34:13 +0000 From: Al Viro To: Martin Brandenburg Cc: Mike Marshall , Linus Torvalds , linux-fsdevel Subject: Re: Orangefs ABI documentation Message-ID: <20160130173413.GE17997@ZenIV.linux.org.uk> References: <20160123001202.GJ17997@ZenIV.linux.org.uk> <20160123012808.GK17997@ZenIV.linux.org.uk> <20160123191055.GN17997@ZenIV.linux.org.uk> <20160123214006.GO17997@ZenIV.linux.org.uk> <20160124001615.GT17997@ZenIV.linux.org.uk> <20160124040529.GX17997@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, Jan 26, 2016 at 02:52:23PM -0500, Martin Brandenburg wrote: > On 1/23/16, Al Viro wrote: > > We'll just decrement the refcount of bufmap, do nothing since it hasn't > > reached zero, proceed to mark all ops as purged, wake each service_operation() > > up and sod off. Now, the holders of those slots will call > > orangefs_get_bufmap_init(), get 1 (since we hadn't dropped the last reference > > yet - can it *ever* see 0 there, actually?) and return -EAGAIN. With > > wait_for_direct_io() noticing that, freeing the slot and going into restart. > > And if there was the only one, we are fine, but what if there were several? > > The answer here is yes. Otherwise a malicious client could not set up > the bufmap then crash the kernel by attempting to use it. Umm... The question was "what happens if there was more than one slot in use when we hit if (ret == -EAGAIN && op_state_purged(new_op)) { orangefs_bufmap_put(bufmap, buffer_index); gossip_debug(GOSSIP_FILE_DEBUG, "%s:going to repopulate_shared_memory.\n", __func__); goto populate_shared_memory; } in wait_for_direct_io()?" If the answer is "yes", I'd like to see more detailed version, if possible... Note that __orangefs_bufmap won't become NULL until all slots are freed, so getting to that place with more than one slot in use will have us go to populate_shared_memory, where we'll grab a new reference to the same old bufmap and allocate a slot _there_... And again, how could /* op uses shared memory */ if (orangefs_get_bufmap_init() == 0) { in service_operation() possibly be true, when we have * op->uses_shared_memory just checked to be set * all callers that set it (orangefs_readdir() and wait_for_direct_io() having allocated a slot before calling service_operation() and not releasing it until service_operation() returns * __orangefs_bufmap not becoming NULL until all slots are freed and * orangefs_get_bufmap_init() returning 1 unless __orangefs_bufmap is NULL? AFAICS, that code (waiting for daemon to be restarted) is provably never executed. What am I missing?