From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from fieldses.org ([174.143.236.118]:60210 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751076Ab0IGFN0 (ORCPT ); Tue, 7 Sep 2010 01:13:26 -0400 Date: Tue, 7 Sep 2010 01:12:41 -0400 From: "J. Bruce Fields" To: Trond Myklebust Cc: linux-nfs@vger.kernel.org Subject: [PATCH] Fix race corrupting rpc upcall list Message-ID: <20100907051241.GB14584@fieldses.org> References: <20100828170953.GB5104@fieldses.org> <20100830175728.GA18764@fieldses.org> <20100907050142.GA14584@fieldses.org> Content-Type: text/plain; charset=us-ascii In-Reply-To: <20100907050142.GA14584@fieldses.org> Sender: linux-nfs-owner@vger.kernel.org List-ID: MIME-Version: 1.0 From: J. Bruce Fields 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. Signed-off-by: J. Bruce Fields --- diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c index 36eee66..8ad9a34 100644 --- a/net/sunrpc/auth_gss/auth_gss.c +++ b/net/sunrpc/auth_gss/auth_gss.c @@ -744,23 +744,6 @@ static int gss_pipe_open_v1(struct inode *inode) static void gss_pipe_release(struct inode *inode) { - struct rpc_inode *rpci = RPC_I(inode); - struct gss_upcall_msg *gss_msg; - - spin_lock(&inode->i_lock); - while (!list_empty(&rpci->in_downcall)) { - - gss_msg = list_entry(rpci->in_downcall.next, - struct gss_upcall_msg, list); - gss_msg->msg.errno = -EPIPE; - atomic_inc(&gss_msg->count); - __gss_unhash_msg(gss_msg); - spin_unlock(&inode->i_lock); - gss_release_msg(gss_msg); - spin_lock(&inode->i_lock); - } - spin_unlock(&inode->i_lock); - put_pipe_version(); }