linux-kernel - Re: [GIT PULL] EFI changes for v4.2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150602135944.GB29523@redhat.com>
Date:	Tue, 2 Jun 2015 09:59:44 -0400
From:	Peter Jones <pjones@...hat.com>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	Matt Fleming <matt@...eblueprint.co.uk>,
	"H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] EFI changes for v4.2

On Tue, Jun 02, 2015 at 08:45:57AM +0200, Ingo Molnar wrote:
> @@ -167,7 +167,6 @@ static struct kset *esrt_kset;
>  
>  static int esre_create_sysfs_entry(void *esre, int entry_num)
>  {
> -       int rc = 0;
>         struct esre_entry *entry;
>         char name[20];
>  
> @@ -180,13 +179,15 @@ static int esre_create_sysfs_entry(void *esre, int entry_num)
>         entry->kobj.kset = esrt_kset;
>  
>         if (esrt->fw_resource_version == 1) {
> +               int rc = 0;
> +
>                 entry->esre.esre1 = esre;
>                 rc = kobject_init_and_add(&entry->kobj, &esre1_ktype, NULL,
>                                           "%s", name);
> -       }
> -       if (rc) {
> -               kfree(entry);
> -               return rc;
> +               if (rc) {
> +                       kfree(entry);
> +                       return rc;
> +               }
>         }
>  
>         list_add_tail(&entry->list, &entry_list);
> 
> How can a compiler ever have warned about 'rc' being uninitialized? It's defined 
> straight at function entry, with initialization to 0. It can never be 
> uninitialized.
>
> I pulled it, because I agree with the change itself, as it's always better to 
> define and use variables in the narrowest scope possible, but I think it's a 
> cleanup, not a compiler warning fix.

Well, apparently I failed to explain it well - the warning was about
"esre" rather than "rc".  Basically before we were testing the version in
register_entries() (i.e. this function's caller) and never calling the
this function if it's not version 1.  The compiler didn't figure out
that when we set "entry->esre.esre1 = esre;", esre can not be null
because the function wouldn't be called.  Adding the explicit check
on the version here silenced the warning about entry plausibly being
NULL.

I'm guessing that this is because it's checking that the same
conditional test is involved - that the initialization is in the same
"...version == 1" test that the usage is.  But that's just a guess.

Would you like another patch to add this email to the commit message, or
do you want to add it in your tree, or what?

-- 
        Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/