diff mbox series

[4/4] alpha: Use absolute_pointer for strcmp on fixed memory location

Message ID 20210912160149.2227137-5-linux@roeck-us.net (mailing list archive)
State New
Headers show
Series Introduce and use absolute_pointer macro | expand

Commit Message

Guenter Roeck Sept. 12, 2021, 4:01 p.m. UTC
alpha:allmodconfig fails to build with the following error
when using gcc 11.x.

arch/alpha/kernel/setup.c: In function 'setup_arch':
arch/alpha/kernel/setup.c:493:13: error:
	'strcmp' reading 1 or more bytes from a region of size 0

Avoid the problem by using absolute_pointer() when providing a memory
address to strcmp().

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 arch/alpha/kernel/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Linus Torvalds Sept. 12, 2021, 7:13 p.m. UTC | #1
On Sun, Sep 12, 2021 at 9:02 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> -       if (strcmp(COMMAND_LINE, "INSTALL") == 0) {
> +       if (strcmp(absolute_pointer(COMMAND_LINE), "INSTALL") == 0) {

Again, this feels very much like treating the symptoms, not actually
fixing the real issue.

It's COMMAND_LINE itself that should have been fixed up, not that one use of it.

Because the only reason you didn't get a warning from later uses is
that 'strcmp()' is doing compiler-specific magic. You're just delaying
the inevitable warnings about the other uses of that thing.

            Linus
Guenter Roeck Sept. 12, 2021, 8:15 p.m. UTC | #2
On 9/12/21 12:13 PM, Linus Torvalds wrote:
> On Sun, Sep 12, 2021 at 9:02 AM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> -       if (strcmp(COMMAND_LINE, "INSTALL") == 0) {
>> +       if (strcmp(absolute_pointer(COMMAND_LINE), "INSTALL") == 0) {
> 
> Again, this feels very much like treating the symptoms, not actually
> fixing the real issue.
> 
> It's COMMAND_LINE itself that should have been fixed up, not that one use of it.
> 
> Because the only reason you didn't get a warning from later uses is
> that 'strcmp()' is doing compiler-specific magic. You're just delaying
> the inevitable warnings about the other uses of that thing.
> 

Makes sense. I'll change the patch accordingly.

Thanks,
Guenter
Guenter Roeck Sept. 12, 2021, 8:37 p.m. UTC | #3
On 9/12/21 12:13 PM, Linus Torvalds wrote:
> On Sun, Sep 12, 2021 at 9:02 AM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> -       if (strcmp(COMMAND_LINE, "INSTALL") == 0) {
>> +       if (strcmp(absolute_pointer(COMMAND_LINE), "INSTALL") == 0) {
> 
> Again, this feels very much like treating the symptoms, not actually
> fixing the real issue.
> 
> It's COMMAND_LINE itself that should have been fixed up, not that one use of it.
> 
> Because the only reason you didn't get a warning from later uses is
> that 'strcmp()' is doing compiler-specific magic. You're just delaying
> the inevitable warnings about the other uses of that thing.
> 

COMMAND_LINE is, for whatever reason, defined in
arch/alpha/include/uapi/asm/setup.h, ie in the uapi.

Can I even touch that ?

Guenter
Linus Torvalds Sept. 12, 2021, 10:58 p.m. UTC | #4
On Sun, Sep 12, 2021 at 1:37 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> COMMAND_LINE is, for whatever reason, defined in
> arch/alpha/include/uapi/asm/setup.h, ie in the uapi.
>
> Can I even touch that ?

I think that's entirely a historical accident.

Note how most of those #define's don't even work for user space
because they use PAGE_OFFSET, which is defined in <asm/page.h>. And
others depend on the kernel config system.

There's a couple that do seem to be potentially for user space (MILO
bootloader), and who knows just what hacks that code might have with
internal knowledge of this header file. But anything I can find on the
net seems to predate our move to 'uapi' headers, so I wouldn't really
worry about it.

So I'd suggest just moving that whole file back to <asm/setup.h>,
changing it as necessary, and then seeing if anybody notices.

Because I suspect the answer to that is just crickets chirping..

              Linus
diff mbox series

Patch

diff --git a/arch/alpha/kernel/setup.c b/arch/alpha/kernel/setup.c
index b4fbbba30aa2..aab477a76c30 100644
--- a/arch/alpha/kernel/setup.c
+++ b/arch/alpha/kernel/setup.c
@@ -490,7 +490,7 @@  setup_arch(char **cmdline_p)
 	/* Hack for Jensen... since we're restricted to 8 or 16 chars for
 	   boot flags depending on the boot mode, we need some shorthand.
 	   This should do for installation.  */
-	if (strcmp(COMMAND_LINE, "INSTALL") == 0) {
+	if (strcmp(absolute_pointer(COMMAND_LINE), "INSTALL") == 0) {
 		strlcpy(command_line, "root=/dev/fd0 load_ramdisk=1", sizeof command_line);
 	} else {
 		strlcpy(command_line, COMMAND_LINE, sizeof command_line);