From patchwork Wed Apr 12 17:43:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sumit Semwal X-Patchwork-Id: 97327 Delivered-To: patch@linaro.org Received: by 10.140.109.52 with SMTP id k49csp374295qgf; Wed, 12 Apr 2017 10:44:19 -0700 (PDT) X-Received: by 10.98.213.138 with SMTP id d132mr4467233pfg.172.1492019059047; Wed, 12 Apr 2017 10:44:19 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id j29si17209540pfj.125.2017.04.12.10.44.18; Wed, 12 Apr 2017 10:44:19 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org; spf=pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752057AbdDLRoQ (ORCPT + 6 others); Wed, 12 Apr 2017 13:44:16 -0400 Received: from mail-pg0-f45.google.com ([74.125.83.45]:34673 "EHLO mail-pg0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755347AbdDLRoN (ORCPT ); Wed, 12 Apr 2017 13:44:13 -0400 Received: by mail-pg0-f45.google.com with SMTP id 21so18163040pgg.1 for ; Wed, 12 Apr 2017 10:44:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=q4MHM8Ry+qyRFBf2kgtGxjSEJ0pWHgg8MgH/QgFJLi0=; b=HY2GAYSmCszi6oT7sf5nuOjYbT5jrx5zj3xSmpWqUmNvcLi2Uh5On/aFNudmITHq0n D2iCEKyhocr53870rrPTEzCdxtXEPFmcb6n7z7fmw0uzZOnydHNmW9b4bSRCJhCMMCHS 0FkrpCHnZ3k7Y+naODfodSzW9HfhX141ml1pk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=q4MHM8Ry+qyRFBf2kgtGxjSEJ0pWHgg8MgH/QgFJLi0=; b=F8Yg+0ixvMSCUUMWsnkVUpmr81jaau1Knf3/IHGsi/oy3aVNITQN1/A52/dpN1CsRE hj+UcWxiccXIs+uSm4d7FHxPCRAXgD9z/TeNMjAtprncT1A4b0APsRNVQVEWLsT2BR21 14yN04rfxkL0JoUoNPnEtyouzkxzFVWenMTu86MhgUHfV6yKzdKp2R6OUCZ6AXUynBnj V8sLAhws/JzStOCvK9Cyi1Bkuje0L9CfpO05lWq2CHzSI0oQ43oLF46Rq0lQ+vJCgZ0n dr2fd2rG4DmW2odvnkhMwhzDed1hMca9rrJ8VwVjy9C/u+v9Pz2Q0UiwXb6BWHRyLpEp 6fjA== X-Gm-Message-State: AN3rC/4rVFV8xNv+NU0h1y8LLylmH0eHV5jLzItw5D9V7jBkGES2rtXsqktnOWv+WPFY+36K X-Received: by 10.84.238.194 with SMTP id l2mr36248530pln.7.1492019052632; Wed, 12 Apr 2017 10:44:12 -0700 (PDT) Received: from phantom.lan ([106.51.225.38]) by smtp.gmail.com with ESMTPSA id r17sm37750019pfa.13.2017.04.12.10.44.10 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 12 Apr 2017 10:44:11 -0700 (PDT) From: Sumit Semwal To: stable@vger.kernel.org Cc: NeilBrown , Trond Myklebust , Sumit Semwal Subject: [v2 PATCH for-4.4 04/16] SUNRPC: fix refcounting problems with auth_gss messages. Date: Wed, 12 Apr 2017 23:13:38 +0530 Message-Id: <1492019030-13567-5-git-send-email-sumit.semwal@linaro.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1492019030-13567-1-git-send-email-sumit.semwal@linaro.org> References: <1492019030-13567-1-git-send-email-sumit.semwal@linaro.org> Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: NeilBrown [ Upstream commit 1cded9d2974fe4fe339fc0ccd6638b80d465ab2c ] There are two problems with refcounting of auth_gss messages. First, the reference on the pipe->pipe list (taken by a call to rpc_queue_upcall()) is not counted. It seems to be assumed that a message in pipe->pipe will always also be in pipe->in_downcall, where it is correctly reference counted. However there is no guaranty of this. I have a report of a NULL dereferences in rpc_pipe_read() which suggests a msg that has been freed is still on the pipe->pipe list. One way I imagine this might happen is: - message is queued for uid=U and auth->service=S1 - rpc.gssd reads this message and starts processing. This removes the message from pipe->pipe - message is queued for uid=U and auth->service=S2 - rpc.gssd replies to the first message. gss_pipe_downcall() calls __gss_find_upcall(pipe, U, NULL) and it finds the *second* message, as new messages are placed at the head of ->in_downcall, and the service type is not checked. - This second message is removed from ->in_downcall and freed by gss_release_msg() (even though it is still on pipe->pipe) - rpc.gssd tries to read another message, and dereferences a pointer to this message that has just been freed. I fix this by incrementing the reference count before calling rpc_queue_upcall(), and decrementing it if that fails, or normally in gss_pipe_destroy_msg(). It seems strange that the reply doesn't target the message more precisely, but I don't know all the details. In any case, I think the reference counting irregularity became a measureable bug when the extra arg was added to __gss_find_upcall(), hence the Fixes: line below. The second problem is that if rpc_queue_upcall() fails, the new message is not freed. gss_alloc_msg() set the ->count to 1, gss_add_msg() increments this to 2, gss_unhash_msg() decrements to 1, then the pointer is discarded so the memory never gets freed. Fixes: 9130b8dbc6ac ("SUNRPC: allow for upcalls for same uid but different gss service") Cc: stable@vger.kernel.org Link: https://bugzilla.opensuse.org/show_bug.cgi?id=1011250 Signed-off-by: NeilBrown Signed-off-by: Trond Myklebust Signed-off-by: Sumit Semwal --- net/sunrpc/auth_gss/auth_gss.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) -- 2.7.4 diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c index 06095cc..1f0687d 100644 --- a/net/sunrpc/auth_gss/auth_gss.c +++ b/net/sunrpc/auth_gss/auth_gss.c @@ -541,9 +541,13 @@ gss_setup_upcall(struct gss_auth *gss_auth, struct rpc_cred *cred) return gss_new; gss_msg = gss_add_msg(gss_new); if (gss_msg == gss_new) { - int res = rpc_queue_upcall(gss_new->pipe, &gss_new->msg); + int res; + atomic_inc(&gss_msg->count); + res = rpc_queue_upcall(gss_new->pipe, &gss_new->msg); if (res) { gss_unhash_msg(gss_new); + atomic_dec(&gss_msg->count); + gss_release_msg(gss_new); gss_msg = ERR_PTR(res); } } else @@ -836,6 +840,7 @@ gss_pipe_destroy_msg(struct rpc_pipe_msg *msg) warn_gssd(); gss_release_msg(gss_msg); } + gss_release_msg(gss_msg); } static void gss_pipe_dentry_destroy(struct dentry *dir,