From patchwork Thu Feb 9 07:38:21 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Windsor X-Patchwork-Id: 9567865 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 3D2CE60231 for ; Sat, 11 Feb 2017 06:23:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 29EE6285A8 for ; Sat, 11 Feb 2017 06:23:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1AF06285A7; Sat, 11 Feb 2017 06:23:51 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.9 required=2.0 tests=BAYES_00, DATE_IN_PAST_24_48, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,FREEMAIL_FROM,RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A6C06285A7 for ; Sat, 11 Feb 2017 06:23:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750747AbdBKGXt (ORCPT ); Sat, 11 Feb 2017 01:23:49 -0500 Received: from mail-yw0-f193.google.com ([209.85.161.193]:35594 "EHLO mail-yw0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750738AbdBKGXs (ORCPT ); Sat, 11 Feb 2017 01:23:48 -0500 Received: by mail-yw0-f193.google.com with SMTP id l16so3932714ywb.2; Fri, 10 Feb 2017 22:23:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=nJpOrvLZ9OB5ApbrzrF0d85BlIjPrLIHOXEeU8y0WYg=; b=VZ4H1P+vvYYulTTUGbF+ApPAywhuURgayAZIR0TXI2WkOBaFUwSp8q/ORrLY3pSYgQ Uc/IsprN7DY9AOUk3/1V39qqifRDRZPjIJFOUUVs5IXnKG77YPc6i6/urwzPOw144sSk /rdSpuH7KnvUk4KbHZkBWKHevDoAJFVH2uiy42dm9ajyM99jbugzJlprnQPeyrApZxl3 k1/4Vx1z6m+BVeuGvFtAdGOUAEi6+8e4yG1M7qkfi+/rFn11m/HqOeh3inIcsVNpR7gO f6RuRZBglDyf2O3xjjYPyDUMR7fy4khH67QL9b+XBYrtfhQZ6vzKBGiz4qUULgLDvZbX GLoA== 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; bh=nJpOrvLZ9OB5ApbrzrF0d85BlIjPrLIHOXEeU8y0WYg=; b=ENJ8InqvFcUN2fyIAqVq4HpN82nS7rodtBE8SLM+7Vu2YgmhiJtdL1HAZtKeX5AsVz u7z4Vc1EaBldUuuqMnhPsWD0r8l1Tdrf3oHhLP9jyS6FNacWlvfQV0CEsvWDJZs13H3n H6RVUfLDOaLx6XuzUU4MGdHIXBUkT3tsdqJUUSHsmlHxXWJYpz7IafppZ3qFeL/smEzj KYPwweIZeaQJmHJfxW1DkfHEhhxtGgSqQv7GoFGOj6pY5Z40XNUSXbfROScV+eocFFbb p7twsFiAxnpe+NnBzER6gwWV2uXSCzyCMAAD810oLpoqHM8+JdM0dl1HXgM3UHAZi1G0 N/mA== X-Gm-Message-State: AMke39mJ6Jcp+8ohGL0nttU/Lo88y5s3hKUbSDyjv2kgPqnXSKdjyj4j5mIWJUqFJ16b9A== X-Received: by 10.13.247.134 with SMTP id h128mr10661165ywf.56.1486794188638; Fri, 10 Feb 2017 22:23:08 -0800 (PST) Received: from kerndev.lan (173-30-6-249.client.mchsi.com. [173.30.6.249]) by smtp.gmail.com with ESMTPSA id b124sm800442ywd.64.2017.02.10.22.23.07 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 10 Feb 2017 22:23:08 -0800 (PST) From: David Windsor To: linux-nfs@vger.kernel.org, netdev@vger.kernel.org Cc: kernel-hardening@lists.openwall.com, bfields@fieldses.org, jlayton@poochiereds.net, keescook@chromium.org, elena.reshetova@intel.com, dwindsor@gmail.com Subject: [RFC][PATCH] nfsd: add +1 to reference counting scheme for struct nfsd4_session Date: Thu, 9 Feb 2017 02:38:21 -0500 Message-Id: <1486625901-10094-1-git-send-email-dwindsor@gmail.com> X-Mailer: git-send-email 2.7.4 Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP In furtherance of the KSPP effort to add overflow protection to kernel reference counters, a new type (refcount_t) and API have been created. Part of the refcount_t API is refcount_inc(), which will not increment a refcount_t variable if its value is 0 (as this would indicate a possible use-after-free condition). In auditing the kernel for refcounting corner cases, we've come across the case of struct nfsd4_session. From fs/nfsd/state.h: /* * Representation of a v4.1+ session. These are refcounted in a similar * fashion to the nfs4_client. References are only taken when the server * is actively working on the object (primarily during the processing of * compounds). */ struct nfsd4_session { atomic_t se_ref; ... }; From fs/nfsd/nfs4state.c: static void init_session(..., struct nfsd4_session *new, ...) { ... atomic_set(&new->se_ref, 0); ... } Since nfsd4_session objects are initialized with refcount = 0, subsequent increments will fail using the new refcount_t API. Being largely unfamiliar with this subsystem's garbage collection mechanism, I'm unsure how to best fix this. Attached is a patch that performs a logical +1 on struct nfsd4_session's reference counting scheme. If this is the correct route to take, I will resubmit this patch with updated comments for how struct nfsd4_session is refcounted (see the above comment from fs/nsfd/state.h). This is in preparation for the previously mentioned refcount_t API series. Thanks, David Windsor Signed-off-by: David Windsor --- fs/nfsd/nfs4state.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index a0dee8a..b0f3010 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -196,7 +196,7 @@ static void nfsd4_put_session_locked(struct nfsd4_session *ses) lockdep_assert_held(&nn->client_lock); - if (atomic_dec_and_test(&ses->se_ref) && is_session_dead(ses)) + if (!atomic_add_unless(&ses->se_ref, -1, 1) && is_session_des(ses)) free_session(ses); put_client_renew_locked(clp); } @@ -1645,7 +1645,7 @@ static void init_session(struct svc_rqst *rqstp, struct nfsd4_session *new, stru new->se_flags = cses->flags; new->se_cb_prog = cses->callback_prog; new->se_cb_sec = cses->cb_sec; - atomic_set(&new->se_ref, 0); + atomic_set(&new->se_ref, 1); idx = hash_sessionid(&new->se_sessionid); list_add(&new->se_hash, &nn->sessionid_hashtbl[idx]); spin_lock(&clp->cl_lock); @@ -1792,7 +1792,7 @@ free_client(struct nfs4_client *clp) ses = list_entry(clp->cl_sessions.next, struct nfsd4_session, se_perclnt); list_del(&ses->se_perclnt); - WARN_ON_ONCE(atomic_read(&ses->se_ref)); + WARN_ON_ONCE((atomic_read(&ses->se_ref) > 1)); free_session(ses); } rpc_destroy_wait_queue(&clp->cl_cb_waitq);