From patchwork Mon Aug 24 11:30:14 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ondrej Mosnacek X-Patchwork-Id: 11732899 X-Patchwork-Delegate: paul@paul-moore.com Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 5463F16B1 for ; Mon, 24 Aug 2020 11:32:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3BCBA2078D for ; Mon, 24 Aug 2020 11:32:39 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="aGPXla08" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726943AbgHXLan (ORCPT ); Mon, 24 Aug 2020 07:30:43 -0400 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:22980 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726337AbgHXLah (ORCPT ); Mon, 24 Aug 2020 07:30:37 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1598268623; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=jRbF9eqt+fCBbJGPV0nNnxFNxJFBANrDtCOL4UOfKs4=; b=aGPXla08jbrGUYBsfJU3P9G15p1MSr10vHeBt0CYOYIST/bajjyS4HeaYsXq4k/bHGRgU+ xz3fTERALwNAXVVlNaGbeFEYWBRFWJLnnImw8ZJGe7NHftgKfF2iPzd/qatpoKTYtIXGXE /O/7DQr9GXsgpPuYDQWvowmR8EBKiJE= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-390-W0ns5G5GNya6d02XSM08UA-1; Mon, 24 Aug 2020 07:30:20 -0400 X-MC-Unique: W0ns5G5GNya6d02XSM08UA-1 Received: by mail-wr1-f70.google.com with SMTP id g3so3717575wrx.1 for ; Mon, 24 Aug 2020 04:30:20 -0700 (PDT) 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:mime-version:content-transfer-encoding; bh=jRbF9eqt+fCBbJGPV0nNnxFNxJFBANrDtCOL4UOfKs4=; b=gn+ec8ot8TKQmLRq9dinADsioHtSpgLYKaEBoKV1E+ZS/Uhu/WGN5jdIgGwbh0cXH9 l3ffSr9vVl/LCXFyyVnZAaZqt3JkpNVO/Kps9bEvszzwt9dtjVHg0RwvvPiNYQjPOA5J OLOVXsQIMM1TSQv/6t60aVfrUGRPbmvyihGryoQoom8xyEhtA8tsBEIOcezNmmXWCp9Q C7vDezBHfR3a3ZaF6WbYoxg9vNw43wRbJLN+XDqGAtwd/ye4etZ0DoIvVf8f0xwQ39dB b2jbDZ+fmi2ZpiKB5KhJ3ZObKVOHrpUaAyUi/TJ+SNS0j2lixcO2Fj4gSCnFk3Jr95Hf vYSw== X-Gm-Message-State: AOAM531TPV8xZGdbLXVkxuT06eh2ZLK2GFGAm2B/bmAHH2zj1Uge3nn4 Xx5sqYrqA4PW8D8SY9jNFU4k/le7AHzQIPBmRtlNk31/vLIr/Ka2xumelJldYWXss42buyqJFm4 u8N6Gud/B4DECrbkDeA== X-Received: by 2002:a1c:dc45:: with SMTP id t66mr5499241wmg.17.1598268618793; Mon, 24 Aug 2020 04:30:18 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwnZb6xSf+SmZ+4Fag6NCRqjC5oelQUR3r8b9Sj1VewgihYa0okDjJE94vxthwcRebW499IBg== X-Received: by 2002:a1c:dc45:: with SMTP id t66mr5499218wmg.17.1598268618601; Mon, 24 Aug 2020 04:30:18 -0700 (PDT) Received: from omos.redhat.com ([2a02:8308:b103:4000:e83d:a4fb:e589:6902]) by smtp.gmail.com with ESMTPSA id l1sm25092434wrb.12.2020.08.24.04.30.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 Aug 2020 04:30:18 -0700 (PDT) From: Ondrej Mosnacek To: selinux@vger.kernel.org, Paul Moore Cc: Stephen Smalley Subject: [PATCH 1/2] selinux: fix a race condition in security_read_policy() Date: Mon, 24 Aug 2020 13:30:14 +0200 Message-Id: <20200824113015.1375857-2-omosnace@redhat.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200824113015.1375857-1-omosnace@redhat.com> References: <20200824113015.1375857-1-omosnace@redhat.com> MIME-Version: 1.0 Sender: selinux-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: selinux@vger.kernel.org In security_read_policy(), the policy length is computed using security_policydb_len(), which does a separate transaction, and then another transaction is done to write the policydb into a buffer of this length. The bug is that the policy might be re-loaded in between the two transactions and so the length can be wrong. In case the new length is lower than the old length, the length is corrected at the end of the function. In case the new length is higher than the old one, an error is returned. Since we can't call vmalloc_user() under read_lock(), fix it by checking if the allocated buffer is sufficiently large after doing the allocation and taking the read lock and if not, retry the whole operation with the new size. Fixes: cee74f47a6ba ("SELinux: allow userspace to read policy back out of the kernel") Signed-off-by: Ondrej Mosnacek --- security/selinux/ss/services.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index a48fc1b337ba9..2c9072f095985 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -3849,14 +3849,22 @@ int security_read_policy(struct selinux_state *state, *len = security_policydb_len(state); +again: *data = vmalloc_user(*len); if (!*data) return -ENOMEM; + read_lock(&state->ss->policy_rwlock); + if (*len < state->ss->policy->policydb.len) { + *len = state->ss->policy->policydb.len; + read_unlock(&state->ss->policy_rwlock); + vfree(*data); + goto again; + } + fp.data = *data; fp.len = *len; - read_lock(&state->ss->policy_rwlock); rc = policydb_write(&state->ss->policy->policydb, &fp); read_unlock(&state->ss->policy_rwlock); From patchwork Mon Aug 24 11:30:15 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ondrej Mosnacek X-Patchwork-Id: 11732901 X-Patchwork-Delegate: paul@paul-moore.com Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 011F81751 for ; Mon, 24 Aug 2020 11:35:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D235F20774 for ; Mon, 24 Aug 2020 11:35:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="LRsJPlra" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726624AbgHXLaj (ORCPT ); Mon, 24 Aug 2020 07:30:39 -0400 Received: from us-smtp-delivery-1.mimecast.com ([207.211.31.120]:51905 "EHLO us-smtp-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726889AbgHXLae (ORCPT ); Mon, 24 Aug 2020 07:30:34 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1598268622; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=YE/jK4eKX/v8UKDjgiKwa3k55ce8EcYH7DtpXZaxArA=; b=LRsJPlraSZuZGxRe87QOPCCktZoRJMRF3OMJ8/49x0pD/FuBg4YC9tsL0FdLyVmia27bNS V/+gpyWRaJSTr1ogpl0/0iyFJSoBJUJnORYUukrco3FKJQYrIgVi+rJb7Mu8pk5h2UsGD0 mfytJ+ANkrjsAeENi4KMvrxqLwR3O9k= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-255-GwF-bCrbNh-7NV0-Btcjuw-1; Mon, 24 Aug 2020 07:30:21 -0400 X-MC-Unique: GwF-bCrbNh-7NV0-Btcjuw-1 Received: by mail-wr1-f72.google.com with SMTP id b8so3873470wrr.2 for ; Mon, 24 Aug 2020 04:30:20 -0700 (PDT) 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:mime-version:content-transfer-encoding; bh=YE/jK4eKX/v8UKDjgiKwa3k55ce8EcYH7DtpXZaxArA=; b=LH9+3x6CpUbJmSPxN6laenKq04eq4wFFxgX247O2DRm/3oMJYRhXsBBRl2i3IjIpZR 98x8GviE9bxhGRWZSP7P1GSZ+k0wCOLRq9E44nj8xfsODl6A4RvfdBsxhj2WVLeP21eb /0ciyBRmM9dDpS+XHLxJlbkzzztYjZSNuz8mVOf7G3SQd5SnsX287wj1GTscGdw6oJp1 vm3NW5y6BLJthlVwhS9FGEgGO/MIm0iADicq0NLWORVxzsMIG7Tj4Xfd1wUelkWwxX0v lds2BoVqhq2Mx3b0U4An5qtYCHXn2TMxK/GI1zeisJOmio4XXNuIfW9tYI+Ciy1kcL98 Lq1g== X-Gm-Message-State: AOAM5316gvChwrmewFs1tBKX2VAV51Bdx52qi2lNpYYVTMzN/Aio00qn pLu3uv8f7HdtK374EfvsoL2bPwSSTcVTMSJKtOWPzVs7p6wUQr5BUDAFN3514oA3I1H1r43GobH feqUOO8UQjFX6R0rFOw== X-Received: by 2002:a7b:cb19:: with SMTP id u25mr5005349wmj.113.1598268619659; Mon, 24 Aug 2020 04:30:19 -0700 (PDT) X-Google-Smtp-Source: ABdhPJy/Nu8mrM9RbArzyDbTY6Musx3YwYplNfadPgaAy2aJOgH/Nhc23+ISg9RRdPLGzsZS9ItQTA== X-Received: by 2002:a7b:cb19:: with SMTP id u25mr5005325wmj.113.1598268619453; Mon, 24 Aug 2020 04:30:19 -0700 (PDT) Received: from omos.redhat.com ([2a02:8308:b103:4000:e83d:a4fb:e589:6902]) by smtp.gmail.com with ESMTPSA id l1sm25092434wrb.12.2020.08.24.04.30.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 Aug 2020 04:30:18 -0700 (PDT) From: Ondrej Mosnacek To: selinux@vger.kernel.org, Paul Moore Cc: Stephen Smalley Subject: [PATCH 2/2] selinux: fix a race condition in sel_open_policy() Date: Mon, 24 Aug 2020 13:30:15 +0200 Message-Id: <20200824113015.1375857-3-omosnace@redhat.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200824113015.1375857-1-omosnace@redhat.com> References: <20200824113015.1375857-1-omosnace@redhat.com> MIME-Version: 1.0 Sender: selinux-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: selinux@vger.kernel.org The code to update the policy inode size is racy and inefficient. Move it below the security_read_policy() call where we already know the length of the policy we are returning. Since after this, security_policydb_len() is only called from security_load_policy(), remove it and just open-code it there. Fixes: cee74f47a6ba ("SELinux: allow userspace to read policy back out of the kernel") Signed-off-by: Ondrej Mosnacek --- security/selinux/include/security.h | 1 - security/selinux/selinuxfs.c | 12 ++++++------ security/selinux/ss/services.c | 18 +++--------------- 3 files changed, 9 insertions(+), 22 deletions(-) diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index c68ed2beadff4..2c14d4165d688 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -219,7 +219,6 @@ void selinux_policy_cancel(struct selinux_state *state, struct selinux_policy *policy); int security_read_policy(struct selinux_state *state, void **data, size_t *len); -size_t security_policydb_len(struct selinux_state *state); int security_policycap_supported(struct selinux_state *state, unsigned int req_cap); diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 131816878e503..098d012cf40d8 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -403,16 +403,16 @@ static int sel_open_policy(struct inode *inode, struct file *filp) if (!plm) goto err; - if (i_size_read(inode) != security_policydb_len(state)) { - inode_lock(inode); - i_size_write(inode, security_policydb_len(state)); - inode_unlock(inode); - } - rc = security_read_policy(state, &plm->data, &plm->len); if (rc) goto err; + if ((size_t)i_size_read(inode) != plm->len) { + inode_lock(inode); + i_size_write(inode, plm->len); + inode_unlock(inode); + } + fsi->policy_opened = 1; filp->private_data = plm; diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 2c9072f095985..0745d4f3a5765 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2289,20 +2289,6 @@ err: return rc; } -size_t security_policydb_len(struct selinux_state *state) -{ - size_t len; - - if (!selinux_initialized(state)) - return 0; - - read_lock(&state->ss->policy_rwlock); - len = state->ss->policy->policydb.len; - read_unlock(&state->ss->policy_rwlock); - - return len; -} - /** * security_port_sid - Obtain the SID for a port. * @protocol: protocol number @@ -3847,7 +3833,9 @@ int security_read_policy(struct selinux_state *state, if (!selinux_initialized(state)) return -EINVAL; - *len = security_policydb_len(state); + read_lock(&state->ss->policy_rwlock); + *len = state->ss->policy->policydb.len; + read_unlock(&state->ss->policy_rwlock); again: *data = vmalloc_user(*len);