[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAK8P3a2SFStvycD4CX+4xtogaoiDXWB0tpeCDKNsYfxD9nDQcw@mail.gmail.com>
Date: Mon, 20 Nov 2017 15:00:37 +0100
From: Arnd Bergmann <arnd@...db.de>
To: John Johansen <john.johansen@...onical.com>
Cc: James Morris <james.l.morris@...cle.com>,
"Serge E. Hallyn" <serge@...lyn.com>,
Kees Cook <keescook@...omium.org>,
Stephen Rothwell <sfr@...b.auug.org.au>,
Seth Arnold <seth.arnold@...onical.com>,
Michal Hocko <mhocko@...e.com>,
Vlastimil Babka <vbabka@...e.cz>,
LSM List <linux-security-module@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] apparmor: initialized returned struct aa_perms
On Mon, Sep 25, 2017 at 4:29 PM, John Johansen
<john.johansen@...onical.com> wrote:
> On 09/15/2017 03: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 <arnd@...db.de>
>
> I've pulled this into apparmor-next
It apparently never made it into mainline. What happened?
Arnd
Powered by blists - more mailing lists