From patchwork Fri Sep 15 19:55:46 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 112757 Delivered-To: patch@linaro.org Received: by 10.140.106.117 with SMTP id d108csp998746qgf; Fri, 15 Sep 2017 12:56:44 -0700 (PDT) X-Google-Smtp-Source: ADKCNb6ugSFnZPEPnZhsDhYxBzZ1CD3IAQKg+Ytg0fO+d9pE/h0uyyWxXNgPy8DuugRDHmH1a0bh X-Received: by 10.99.157.200 with SMTP id i191mr25179764pgd.247.1505505404833; Fri, 15 Sep 2017 12:56:44 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1505505404; cv=none; d=google.com; s=arc-20160816; b=LgsRGdlHav4VmzUBXVwxnbEh3iXl0867Dwq3mRebinFNPph1aHOQ1eaAdwMihL0hIn GhFQ2S7w+/t8QmylLRp3caIiswndeQOJcR7HHU/Z1QXtGfIPIqsLHHlpwpBI/muhyqJd ApV5NJZoUJ6U4sQrzLwmJ3FwsQqSY0po8DlEGIydmbz9/+dkmrhn6sL+W4jaWISqZO5s eoudNeW+Wv9566gyihjJP1Bq/JI+lxhYfRPUIBF6lCVd2GvmrFQOkHXtFCohb9cEcx0q CZmla1nbe7RTZYNGBg4M01hIR2f5pC7gfqiyMk3ahl89Qnj9tOvY5FU0Ehaj3q6pnyVA 2+eA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:date:subject:cc:to:from :arc-authentication-results; bh=k/s5d7FqBhRHy6C6eqo7BTG+Z+rzWp6oGhf6UGUcNh0=; b=kzdvstC4q7XVypZsPvTR9yO5yQj8ur8wIKfWFxe40I8VjfSNSS9xrmutYkqHMovOTh AWoACIVwIA6W3M6eHBtFonw7qedS0V2fo9ZWgdyc0kS0B3Xg7QTLM0no5EdSVz84D75Y EhZuCgmFksgVsYwjG8bhSfjtHWOAhxgT9p0ztIbaI53MPiBdzfwrXSCY7Y10B42GvhmB Q9YkbbGzU5REwRRtttkUVmpwkYMcf7DyZmTVu4Ey7HJ6U5C6L5kW4lBNRdSh/yi3sQUi E8QOgLNNa+kE1ITt8DYko3JLG+pG2sVRd9IRITOSy3UtNMRmVlNBQ5PwsKmeX27GqvTo 1Clg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id f131si1052345pfc.550.2017.09.15.12.56.44; Fri, 15 Sep 2017 12:56:44 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751883AbdIOT4l (ORCPT + 26 others); Fri, 15 Sep 2017 15:56:41 -0400 Received: from mout.kundenserver.de ([217.72.192.73]:60113 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751542AbdIOT4j (ORCPT ); Fri, 15 Sep 2017 15:56:39 -0400 Received: from wuerfel.lan ([95.208.190.237]) by mrelayeu.kundenserver.de (mreue103 [212.227.15.145]) with ESMTPA (Nemesis) id 0MbK2G-1e9WAe0oxo-00IlLK; Fri, 15 Sep 2017 21:56:24 +0200 From: Arnd Bergmann To: John Johansen , James Morris , "Serge E. Hallyn" Cc: Arnd Bergmann , Kees Cook , Stephen Rothwell , Seth Arnold , Michal Hocko , Vlastimil Babka , linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] apparmor: initialized returned struct aa_perms Date: Fri, 15 Sep 2017 21:55:46 +0200 Message-Id: <20170915195620.1561044-1-arnd@arndb.de> X-Mailer: git-send-email 2.9.0 X-Provags-ID: V03:K0:5qHlCdJxl8SmMIMPH0r3FmuwkhdwpCgk5i2TuTwScrHDlnI/UZD tReVx2+Q+tBGHWUxXlzNVn5IPUyr3zRY/SAnbBiVUPPn57Nj290LvW/vVYnLVuum9b9/Ahi 2ksMbkfEJP8SsBQPjCT/1LKy38txA99EE4ODp54zCa04csclEeUBmlo8zz3ZAx684viEQ8c VFeDvO0bXFCe3V2Wg46dw== X-UI-Out-Filterresults: notjunk:1; V01:K0:ci2A+so39vo=:p4sCcJQUbE/bPDH62K9p4L 1WaVb+FnO25qUQG1q+WmWVVfKGHwcYIjuwVPbiU88EBHiP6ltNZ8CTFYXCarydM7ViwLk4jE6 M5ul67EwUmI8InTDHoAJ7GGzfdzt6darQWX/tkhXk5ncpcUVsiyTuSWZnNd2oGyzcuSPCRq1X EUvjIEGxu94t/Vh5o1ZpORgHCR9QAYLk41YO68K80HXlVdEMshU5spAvaobVyx34t7dVdjiOs psCvmk+03mzie9xwUjJSQiYx5PHnkuQVen+68wpZ63549gU2JaPCRgewElrixJ3fcfNLD9FiR v/hCCJzOUiAc6PMpGkevMZHEwAi1pbVS2AIwqdHRm3PJ/shQkVq8MI5SqFoiH9jnY8iuAcPEU CteJOT5nzUHTOpg78/87BDUeU3hLdPlx3ZSTd2TpqAc0+Db4hcdEnyXrXQAmSS3RiydcuHyX2 VajANZS9ffoJTalLTkVuZd3eA0gIQIHGYKsBWlh/zxU7+5XkKkqTK7mMc1T3qdlH+93AmKYNZ xP7O7J3snhw+7gRAIiMnbO/2NrhJKtLFHkhgXjo8e+ooJvWGG7JU57l8npMHSS0RaFu0WkRny lnb4gXhJybMX/Kx/f5n7cEMX+6LGbBc7rRCTu8UnsK6v3wCZghc4aIC5g2oQuUkKHfn6JIQ5C 2O3n99pKghd7cEu6yMA5HDMq/VIG3NWcd5y9sAdVojpOBPSLT3eNdzumFM9vkBiL+D0Nf4djw X57pC8Jrvg8hAKloH1kXtllHIgWBdx1GeF6lVw== Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org gcc-4.4 points out suspicious code in compute_mnt_perms, where the aa_perms structure is only partially initialized before getting returned: security/apparmor/mount.c: In function 'compute_mnt_perms': security/apparmor/mount.c:227: error: 'perms.prompt' is used uninitialized in this function security/apparmor/mount.c:227: error: 'perms.hide' is used uninitialized in this function security/apparmor/mount.c:227: error: 'perms.cond' is used uninitialized in this function security/apparmor/mount.c:227: error: 'perms.complain' is used uninitialized in this function security/apparmor/mount.c:227: error: 'perms.stop' is used uninitialized in this function security/apparmor/mount.c:227: error: 'perms.deny' is used uninitialized in this function Returning or assigning partially initialized structures is a bit tricky, in particular it is explicitly allowed in c99 to assign a partially intialized structure to another, as long as only members are read that have been initialized earlier. Looking at what various compilers do here, the version that produced the warning copied unintialized stack data, while newer versions (and also clang) either set the other members to zero or don't update the parts of the return buffer that are not modified in the temporary structure, but they never warn about this. In case of apparmor, it seems better to be a little safer and always initialize the aa_perms structure. Most users already do that, this changes the remaining ones, including the one instance that I got the warning for. Fixes: fa488437d0f9 ("apparmor: add mount mediation") Signed-off-by: Arnd Bergmann --- security/apparmor/file.c | 8 +------- security/apparmor/lib.c | 13 +++++-------- security/apparmor/mount.c | 13 ++++++------- 3 files changed, 12 insertions(+), 22 deletions(-) -- 2.9.0 Reviewed-by: Seth Arnold Acked-by: Geert Uytterhoeven diff --git a/security/apparmor/file.c b/security/apparmor/file.c index db80221891c6..86d57e56fabe 100644 --- a/security/apparmor/file.c +++ b/security/apparmor/file.c @@ -227,18 +227,12 @@ static u32 map_old_perms(u32 old) struct aa_perms aa_compute_fperms(struct aa_dfa *dfa, unsigned int state, struct path_cond *cond) { - struct aa_perms perms; - /* FIXME: change over to new dfa format * currently file perms are encoded in the dfa, new format * splits the permissions from the dfa. This mapping can be * done at profile load */ - perms.deny = 0; - perms.kill = perms.stop = 0; - perms.complain = perms.cond = 0; - perms.hide = 0; - perms.prompt = 0; + struct aa_perms perms = { }; if (uid_eq(current_fsuid(), cond->uid)) { perms.allow = map_old_perms(dfa_user_allow(dfa, state)); diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c index 8818621b5d95..6cbc06da964c 100644 --- a/security/apparmor/lib.c +++ b/security/apparmor/lib.c @@ -318,14 +318,11 @@ static u32 map_other(u32 x) void aa_compute_perms(struct aa_dfa *dfa, unsigned int state, struct aa_perms *perms) { - perms->deny = 0; - perms->kill = perms->stop = 0; - perms->complain = perms->cond = 0; - perms->hide = 0; - perms->prompt = 0; - perms->allow = dfa_user_allow(dfa, state); - perms->audit = dfa_user_audit(dfa, state); - perms->quiet = dfa_user_quiet(dfa, state); + *perms = (struct aa_perms) { + .allow = dfa_user_allow(dfa, state), + .audit = dfa_user_audit(dfa, state), + .quiet = dfa_user_quiet(dfa, state), + }; /* for v5 perm mapping in the policydb, the other set is used * to extend the general perm set diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c index 82a64b58041d..ed9b4d0f9f7e 100644 --- a/security/apparmor/mount.c +++ b/security/apparmor/mount.c @@ -216,13 +216,12 @@ static unsigned int match_mnt_flags(struct aa_dfa *dfa, unsigned int state, static struct aa_perms compute_mnt_perms(struct aa_dfa *dfa, unsigned int state) { - struct aa_perms perms; - - perms.kill = 0; - perms.allow = dfa_user_allow(dfa, state); - perms.audit = dfa_user_audit(dfa, state); - perms.quiet = dfa_user_quiet(dfa, state); - perms.xindex = dfa_user_xindex(dfa, state); + struct aa_perms perms = { + .allow = dfa_user_allow(dfa, state), + .audit = dfa_user_audit(dfa, state), + .quiet = dfa_user_quiet(dfa, state), + .xindex = dfa_user_xindex(dfa, state), + }; return perms; }