linux-kernel - Re: [PATCH] x86: fix two sparse warnings in early boot string handling

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]

Date:	Wed, 12 Feb 2014 15:31:05 -0800
From:	"H. Peter Anvin" <hpa@...or.com>
To:	David Rientjes <rientjes@...gle.com>,
	Paul Gortmaker <paul.gortmaker@...driver.com>
CC:	linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, x86@...nel.org,
	Matt Fleming <matt@...sole-pimps.org>
Subject: Re: [PATCH] x86: fix two sparse warnings in early boot string handling

On 02/12/2014 03:14 PM, David Rientjes wrote:
> On Wed, 12 Feb 2014, Paul Gortmaker wrote:
> 
>>> This means there is a strstr() prototype that is visible to 
>>> drivers/firmware/efi/efi-stub-helper.c but fails at linkage because you've 
>>> removed the definition. 
>>
>> Yes, because you suggested removal when you said, in what is
>> now deleted context text:
>>
>>  "I don't see why you can't remove strstr() in 
>>  arch/x86/boot/string.c entirely.  What breaks?"
>>
>> The above answers your question. The eboot.c breaks.  So
>> we can't remove strstr.
>>
> 
> Thanks.
> 
>>> So, again, why would you add a duplicate 
>>> prototype with your patch?
>>
>> I'm sure there is an implicit path to <linux/string.h>
>> which allows eboot.c to see a prototype and hence compile.
>>
> 
> Nope, linux/string.h only declares the prototype when 
> #ifndef __HAVE_ARCH_STRSTR and the 32-bit x86 declaration in 
> include/asm/string_32.h properly does #define __HAVE_ARCH_STRSTR.
> 
> There's also no #include ordering issue here since linux/string.h does 
> #include <asm/string.h> first.
> 
> If you had a real problem here, the build would break.  So I'll renew my 
> original objection: I don't think it's acceptable to add unneeded 
> prototypes because sparse doesn't understand this.
> 

The real answer IMO ought to be that since arch/x86/boot/string.c is now
used separately from boot.h (eboot.c which includes efi-stub-helper.c
does *not* include boot.h) we may have to move those string functions
into a separate header file.

Matt, do you have any opinions here?

I don't think it is appropriate to leave these warnings in the code
"just because".  We have too many live warnings, some of which are real
bugs, and we need to encourage people to address them instead of leaving
them lost in the noise.

	-hpa

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