From patchwork Mon Apr 10 17:44:20 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sumit Semwal X-Patchwork-Id: 97199 Delivered-To: patch@linaro.org Received: by 10.182.246.10 with SMTP id xs10csp1456360obc; Mon, 10 Apr 2017 10:45:30 -0700 (PDT) X-Received: by 10.99.60.77 with SMTP id i13mr58386269pgn.185.1491846330383; Mon, 10 Apr 2017 10:45:30 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id n3si14186684pfk.174.2017.04.10.10.45.30; Mon, 10 Apr 2017 10:45:30 -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 S1750732AbdDJRp3 (ORCPT + 5 others); Mon, 10 Apr 2017 13:45:29 -0400 Received: from mail-pg0-f51.google.com ([74.125.83.51]:34917 "EHLO mail-pg0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752473AbdDJRp0 (ORCPT ); Mon, 10 Apr 2017 13:45:26 -0400 Received: by mail-pg0-f51.google.com with SMTP id 81so109365136pgh.2 for ; Mon, 10 Apr 2017 10:45:21 -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=JxWeXbYeislg3eYSHyVLodJzOlp7kNaNuZ/2BqC6fqw=; b=SI9a47N4zlMTDmAWKQLScWAEtfKKTy/9zWNJJTOZD8UOXTLFLKcnBokscldIYMf5IB t/5zxeJGgNhNe8XeAgAtOAeqZ7/b6QKvhgVFzppYEmBhdetrJSyKMtKWHt6Gn5DQ5ECx GMHB1aIblXtn8RGjKx8lSmNTW0pPUERPPUZsA= 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=JxWeXbYeislg3eYSHyVLodJzOlp7kNaNuZ/2BqC6fqw=; b=sJNHIrRnnk4KNvO9fJueRQV4TvZ9QNydeKyuALHq0OfjL3QdPOfjN44YpnMsPLzpHE +/0heNdBP8Yi8mOxcTjp/2RolJXV+BTDE4wuzjL31DOn8IbcEkPoivBQR6G5tqhxk9XE p8ORtqDHFyHt24ciNMe247/dITpWRIoyECHG8/ThTINR2Auh9sp5hMBlY7Loc87r5hHf UYX/ntdBxHdd6IqExrx3M7taFm/w9VzOxXeTgfej1uou0XGASxpRcDQ5RGT7zqNDzN9e vcXI96BI2woWcupn64INuNQEOaZ+8szPoLtmskS+vRK0ABA+ex93I/nr5+R7SXMXqDVS LSXg== X-Gm-Message-State: AFeK/H3F3bRabiCLfByRoYpLAVmLRbCW5KD1PL5xR/SJtaOBWmsRnd7IZQSZMoZT/Pc6IsCi X-Received: by 10.99.123.75 with SMTP id k11mr56337953pgn.150.1491846320245; Mon, 10 Apr 2017 10:45:20 -0700 (PDT) Received: from phantom.lan ([106.51.225.38]) by smtp.gmail.com with ESMTPSA id y6sm768833pfa.83.2017.04.10.10.45.17 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 10 Apr 2017 10:45:19 -0700 (PDT) From: Sumit Semwal To: stable@vger.kernel.org Cc: NeilBrown , Trond Myklebust , Sumit Semwal Subject: [PATCH for-4.4 04/16] SUNRPC: fix refcounting problems with auth_gss messages. Date: Mon, 10 Apr 2017 23:14:20 +0530 Message-Id: <1491846272-14882-5-git-send-email-sumit.semwal@linaro.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1491846272-14882-1-git-send-email-sumit.semwal@linaro.org> References: <1491846272-14882-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,