Re: [GIT PULL] Block driver changes for 5.20-rc1

Re: [GIT PULL] Block driver changes for 5.20-rc1

[Nick Desaulniers]

+ Linux Toolchains, Nick Clifton (author of
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=ba951afb99912da01a6e8434126b8fac7aa75107)
https://lore.kernel.org/linux-block/CAKwvOdkpNRvD0kDe-j8N0Gkq+1Fdhd6=z-9ROm3gc12Sf0k-Kg@mail.gmail.com/
is the thread for context.

On Wed, Aug 3, 2022 at 10:45 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> I ran with the below and it silences the linker warning mentioned. I do
> also see the below with my build (which aren't new with the option added
> obviously, but not visible since I don't get the other noise):
>
> axboe@r7525 ~/gi/linux-block (test)> time make -j256 -s                      1.886s
> ld: warning: .tmp_vmlinux.kallsyms1 has a LOAD segment with RWX permissions
> ld: warning: .tmp_vmlinux.kallsyms2 has a LOAD segment with RWX permissions
> ld: warning: vmlinux has a LOAD segment with RWX permissions

Not sure yet about these.  Looks like there's linker flags to disable
warnings...but I don't recommend those.
https://github.com/systemd/systemd/commit/b0e5bf0451a6bc94e6e7b2a1de668b75c63f38c8
I wonder if they go away by fixing the boot issues described below?

Otherwise, I think we need to find which section is the problematic
one; I suspect the segment flagged as LOAD is composed of many
sections, with possibly only one or a few that needs permissions
adjusted.

> ld: warning: arch/x86/boot/compressed/efi_thunk_64.o: missing .note.GNU-stack section implies executable stack
> ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker

Right, arch/x86/boot/compressed (and arch/x86/boot/) discard/reset
KBUILD_AFLAGS (via the := assignment operator).

So arch/x86/boot/Makefile and arch/x86/boot/compressed/Makefile also
will need changes similar to the one you did below.

Finally, if those parts of the code actually expect the stack to be
executable (probably depends on some inline asm), I suspect we might
get some kind of fault at runtime (though I don't know how the kernel
handles segment permissions or even uses ELF segments).  Point being
that -Wa,--noexecstack is somewhat a promise that we wont try to
execute data on the stack as if it were code.  -Wa,--execstack is
useful if we need to be able to do so, but we should be careful to
limit that to individual translation units if they really truly need
that.  The linker warning is more so about the current ambiguity since
if the implicit default changes in the future, that could break code.
Better for us to be explicit here, and disable executable stack unless
strictly necessary and only as necessary IMO.  Personally, I think the
current implicit default is wrong, but pragmatically I recognize that
people have been used to the status quo for years, and changing it
could break existing codebases.

If you send the below diff with the two other additions I suggest to
the x86 ML, the x86 maintainers might be able to comment if they're
familiar with any possible cases where the stack is expected to be
executable.

> ld: warning: arch/x86/boot/compressed/vmlinux has a LOAD segment with RWX permissions
> ld: warning: arch/x86/boot/pmjump.o: missing .note.GNU-stack section implies executable stack
> ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker

^ See above.

> ld: warning: arch/x86/boot/setup.elf has a LOAD segment with RWX permissions
>
>
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 7854685c5f25..51824528a026 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -123,6 +123,7 @@ else
>          CHECKFLAGS += -D__x86_64__
>
>          KBUILD_AFLAGS += -m64
> +        KBUILD_AFLAGS += -Wa,--noexecstack
>          KBUILD_CFLAGS += -m64
>
>          # Align jump targets to 1 byte, not the default 16 bytes:
>
> --
> Jens Axboe
>


-- 
Thanks,
~Nick Desaulniers

Re: [GIT PULL] Block driver changes for 5.20-rc1

[Jens Axboe]

On 8/3/22 12:06 PM, Nick Desaulniers wrote:
>> ld: warning: arch/x86/boot/compressed/efi_thunk_64.o: missing .note.GNU-stack section implies executable stack
>> ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker
> 
> Right, arch/x86/boot/compressed (and arch/x86/boot/) discard/reset
> KBUILD_AFLAGS (via the := assignment operator).
> 
> So arch/x86/boot/Makefile and arch/x86/boot/compressed/Makefile also
> will need changes similar to the one you did below.
> 
> Finally, if those parts of the code actually expect the stack to be
> executable (probably depends on some inline asm), I suspect we might
> get some kind of fault at runtime (though I don't know how the kernel
> handles segment permissions or even uses ELF segments).  Point being
> that -Wa,--noexecstack is somewhat a promise that we wont try to
> execute data on the stack as if it were code.  -Wa,--execstack is
> useful if we need to be able to do so, but we should be careful to
> limit that to individual translation units if they really truly need
> that.  The linker warning is more so about the current ambiguity since
> if the implicit default changes in the future, that could break code.
> Better for us to be explicit here, and disable executable stack unless
> strictly necessary and only as necessary IMO.  Personally, I think the
> current implicit default is wrong, but pragmatically I recognize that
> people have been used to the status quo for years, and changing it
> could break existing codebases.
> 
> If you send the below diff with the two other additions I suggest to
> the x86 ML, the x86 maintainers might be able to comment if they're
> familiar with any possible cases where the stack is expected to be
> executable.

I'd be happy for you to take this over, I'm really just the reporter
here...

-- 
Jens Axboe