Re: [PATCH] Fix race corrupting rpc upcall list

["J. Bruce Fields" <bfields@fieldses.org>]

On Sun, Sep 12, 2010 at 07:47:48PM -0400, J. Bruce Fields wrote:
> On Sun, Sep 12, 2010 at 05:07:46PM -0400, Trond Myklebust wrote:
> > On Tue, 2010-09-07 at 01:12 -0400, J. Bruce Fields wrote:
> > > From: J. Bruce Fields <bfields@redhat.com>
> > > 
> > > If rpc_queue_upcall() adds a new upcall to the rpci->pipe list just
> > > after rpc_pipe_release calls rpc_purge_list(), but before it calls
> > > gss_pipe_release (as rpci->ops->release_pipe(inode)), then the latter
> > > will free a message without deleting it from the rpci->pipe list.
> > > 
> > > We will be left with a freed object on the rpc->pipe list.  Most
> > > frequent symptoms are kernel crashes in rpc.gssd system calls on the
> > > pipe in question.
> > > 
> > > We could just add a list_del(&gss_msg->msg.list) here.  But I can see no
> > > reason for doing all this cleanup here; the preceding rpc_purge_list()
> > > should have done the job, except possibly for any newly queued upcalls
> > > as above, which can safely be left to wait for another opener.
> > 
> > Hi Bruce,
> > 
> > Looking again at this issue, I think I see why the ->release_pipe() is
> > needed. While the call to rpc_purge_list() does indeed clear the list of
> > all those messages that are waiting for their upcall to complete, it
> > does nothing for the messages that have successfully been read by the
> > daemon, but are now waiting for a reply.
> 
> Doh!
> 
> > How about something like the patch below instead?
> 
> I read it over, and it looks sensible to me.
> 
> It's also survived a few testing iterations.  I'll give it a few more
> just out of paranoia, but would be surprised if they find the problem
> isn't resolved.

Indeed, no surprises; please pass those patches along whenever you're
ready.

--b.