Re: Orangefs ABI documentation

[Al Viro <viro@ZenIV.linux.org.uk>]

On Tue, Jan 26, 2016 at 02:52:23PM -0500, Martin Brandenburg wrote:
> On 1/23/16, Al Viro <viro@zeniv.linux.org.uk> 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?