linux-kernel - Re: [PATCH] firmware: dmi_scan: Fix ordering of product

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: <1429176638.4386.19.camel@chaos.site>
Date:	Thu, 16 Apr 2015 11:30:38 +0200
From:	Jean Delvare <jdelvare@...e.de>
To:	zhenzhong.duan@...cle.com
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Ben Hutchings <ben@...adent.org.uk>,
	Artem Savkov <artem.savkov@...il.com>,
	Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org>,
	Matt Fleming <matt.fleming@...el.com>
Subject: Re: [PATCH] firmware: dmi_scan: Fix ordering of product_uuid

Le Thursday 16 April 2015 à 16:46 +0800, Zhenzhong Duan a écrit :
> On 2015/4/16 15:09, Jean Delvare wrote:
> > Le Thursday 16 April 2015 à 14:22 +0800, Zhenzhong Duan a écrit :
> >> The basic idea is right, but you ignore the case dmi_walk_early may
> >> fail, though looks impossible when bootup.
> >>
> >> Better to add below for robust.
> >>
> >> @@ -521,6 +521,6 @@ static int __init dmi_present(const u8 *
> >>
> >>                return 0;
> >>            }
> >>        }
> >> +    dmi_ver = 0;
> >>        return 1;
> >>    }
> >>
> > What is the value of this? dmi_ver will never be accessed after this
> > point anyway, as far as I can see.
> Same as above, future commit may not realize you bring this faulty when 
> they want to use dmi_ver.

Why do you think this is "faulty"? The value in dmi_ver is correct
whether dmi_walk_early() succeeded or not. There's no rationale for
resetting dmi_ver on error and not dmi_num, dmi_len and dmi_base. Note
that dmi_smbios3_present() doesn't reset any of these either. These
values are all correct.

If other modules need to check whether DMI was successfully initialized,
they must check dmi_available rather than any of the variables above
(which are all static anyway.)

-- 
Jean Delvare
SUSE L3 Support

--
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/