Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934881AbdIYL3O (ORCPT ); Mon, 25 Sep 2017 07:29:14 -0400 Received: from mail-wr0-f193.google.com ([209.85.128.193]:34830 "EHLO mail-wr0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751798AbdIYL3M (ORCPT ); Mon, 25 Sep 2017 07:29:12 -0400 X-Google-Smtp-Source: AOwi7QCnRQ9AUCTpYB4xIEFGpmxOca2KLTIKq3XPTh3QBeZNLtzYmr7dG7eqGs3ekkuYVxv7tuRrpLjrc6mKj35YAMI= MIME-Version: 1.0 In-Reply-To: <20170915195620.1561044-1-arnd@arndb.de> References: <20170915195620.1561044-1-arnd@arndb.de> From: Geert Uytterhoeven Date: Mon, 25 Sep 2017 13:29:11 +0200 X-Google-Sender-Auth: oNdDkK2PmQ03rcTp5VPTd7-NSaM Message-ID: Subject: Re: [PATCH] apparmor: initialized returned struct aa_perms To: Arnd Bergmann Cc: John Johansen , James Morris , "Serge E. Hallyn" , Kees Cook , Stephen Rothwell , Seth Arnold , Michal Hocko , Vlastimil Babka , linux-security-module@vger.kernel.org, "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2148 Lines: 42 On Fri, Sep 15, 2017 at 9:55 PM, Arnd Bergmann wrote: > 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 Acked-by: Geert Uytterhoeven Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds