linux-hardening.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: s390/defconfigs: set CONFIG_INIT_STACK_NONE=y
@ 2023-05-25 18:40 Kees Cook
  2023-05-26 13:42 ` Heiko Carstens
  0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2023-05-25 18:40 UTC (permalink / raw)
  To: Heiko Carstens, Alexander Gordeev; +Cc: linux-hardening

Hi Heiko,

I had this[1] patch pointed out to me, but I couldn't find any discussion
about it on public lists. Can you give me some background on this? There
haven't been any general workloads identified where this has been
a problem, so I'm curious why this was seen as globally an issue on
s390. The expectation was to use __uninitialized on any variables where
this was noticed as a performance issue, and where the memory safety of
the variable could be proven. Turning it off by default seems like
rather too much, but perhaps there is something unique to s390 I don't
know about. :)

Thanks,

-Kees

[1] https://git.kernel.org/linus/124acbe275040809abfa5fbe6e25790a53d5a161

-- 
Kees Cook                                            @outflux.net

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: s390/defconfigs: set CONFIG_INIT_STACK_NONE=y
  2023-05-25 18:40 s390/defconfigs: set CONFIG_INIT_STACK_NONE=y Kees Cook
@ 2023-05-26 13:42 ` Heiko Carstens
  2023-05-26 16:37   ` Kees Cook
  0 siblings, 1 reply; 4+ messages in thread
From: Heiko Carstens @ 2023-05-26 13:42 UTC (permalink / raw)
  To: Kees Cook; +Cc: Alexander Gordeev, linux-hardening, Sven Schnelle

Hi Kees,

> I had this[1] patch pointed out to me, but I couldn't find any discussion
> about it on public lists. Can you give me some background on this? There
> haven't been any general workloads identified where this has been
> a problem, so I'm curious why this was seen as globally an issue on
> s390. The expectation was to use __uninitialized on any variables where
> this was noticed as a performance issue, and where the memory safety of
> the variable could be proven. Turning it off by default seems like
> rather too much, but perhaps there is something unique to s390 I don't
> know about. :)

This was the result of some micro benchmarks being reported "too slow".
Actually our syscall entry/exit path got naturally slower since we switched
to generic entry; now we are trying to improve things a bit again.

There is also this RFC from Sven, which tries to inline some of the
generic system call functions, in order to avoid function calls:
https://lore.kernel.org/lkml/20230516133810.171487-1-svens@linux.ibm.com/

I stumbled upon CONFIG_INIT_STACK_NONE only by accident when wondering why
the compiler would generate quite some instructions which aren't necessary,
just to zero variables. For the getpid() system call this makes a runtime
difference of ~3%, which is quite a bit.

What is the overhead on other architectures?

That said: I was also unaware of __uninitialized. But on the other hand,
there is no sign of __uninitialized in the kernel, nor could I find
anything that could match in compiler_attributes.h.
Am I missing something here?

Thanks for bringing this up, I guess if there is some annotation available,
we can revisit at least the architecture specific entry code, and maybe
figure out how to avoid most of the extra runtime, but still keep the
feature enabled.

(Adding Sven, since I will be offline the next two weeks).

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: s390/defconfigs: set CONFIG_INIT_STACK_NONE=y
  2023-05-26 13:42 ` Heiko Carstens
@ 2023-05-26 16:37   ` Kees Cook
  2023-05-30  6:44     ` Sven Schnelle
  0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2023-05-26 16:37 UTC (permalink / raw)
  To: Heiko Carstens; +Cc: Alexander Gordeev, linux-hardening, Sven Schnelle

On Fri, May 26, 2023 at 03:42:56PM +0200, Heiko Carstens wrote:
> Hi Kees,
> 
> > I had this[1] patch pointed out to me, but I couldn't find any discussion
> > about it on public lists. Can you give me some background on this? There
> > haven't been any general workloads identified where this has been
> > a problem, so I'm curious why this was seen as globally an issue on
> > s390. The expectation was to use __uninitialized on any variables where
> > this was noticed as a performance issue, and where the memory safety of
> > the variable could be proven. Turning it off by default seems like
> > rather too much, but perhaps there is something unique to s390 I don't
> > know about. :)
> 
> This was the result of some micro benchmarks being reported "too slow".
> Actually our syscall entry/exit path got naturally slower since we switched
> to generic entry; now we are trying to improve things a bit again.
> 
> There is also this RFC from Sven, which tries to inline some of the
> generic system call functions, in order to avoid function calls:
> https://lore.kernel.org/lkml/20230516133810.171487-1-svens@linux.ibm.com/
> 
> I stumbled upon CONFIG_INIT_STACK_NONE only by accident when wondering why
> the compiler would generate quite some instructions which aren't necessary,
> just to zero variables. For the getpid() system call this makes a runtime
> difference of ~3%, which is quite a bit.

Hm, that does seem high. It implies there are large variable that are
being passed by reference, perhaps in the syscall path? I had similar
problems a while back on x86 but due to stack-protector seeing the
register arrays and thinking they needed protection. I had to explicitly
turn that off for the entry code, since they're provably safe. :)

> What is the overhead on other architectures?

It's been in the noise for real workloads.

> That said: I was also unaware of __uninitialized. But on the other hand,
> there is no sign of __uninitialized in the kernel, nor could I find
> anything that could match in compiler_attributes.h.
> Am I missing something here?

No, nothing missed -- there just have been no workloads identified yet
where it's needed.

> Thanks for bringing this up, I guess if there is some annotation available,
> we can revisit at least the architecture specific entry code, and maybe
> figure out how to avoid most of the extra runtime, but still keep the
> feature enabled.
> 
> (Adding Sven, since I will be offline the next two weeks).

Yeah, if you find a place where it's needed, please add the compiler
attribute and put it to use. It'll give people an example use-case for
it. :)

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: s390/defconfigs: set CONFIG_INIT_STACK_NONE=y
  2023-05-26 16:37   ` Kees Cook
@ 2023-05-30  6:44     ` Sven Schnelle
  0 siblings, 0 replies; 4+ messages in thread
From: Sven Schnelle @ 2023-05-30  6:44 UTC (permalink / raw)
  To: Kees Cook; +Cc: Heiko Carstens, Alexander Gordeev, linux-hardening

Hi Kees,

Kees Cook <keescook@chromium.org> writes:

> On Fri, May 26, 2023 at 03:42:56PM +0200, Heiko Carstens wrote:
>> Hi Kees,
>> 
>> > I had this[1] patch pointed out to me, but I couldn't find any discussion
>> > about it on public lists. Can you give me some background on this? There
>> > haven't been any general workloads identified where this has been
>> > a problem, so I'm curious why this was seen as globally an issue on
>> > s390. The expectation was to use __uninitialized on any variables where
>> > this was noticed as a performance issue, and where the memory safety of
>> > the variable could be proven. Turning it off by default seems like
>> > rather too much, but perhaps there is something unique to s390 I don't
>> > know about. :)
>> 
>> This was the result of some micro benchmarks being reported "too slow".
>> Actually our syscall entry/exit path got naturally slower since we switched
>> to generic entry; now we are trying to improve things a bit again.
>> 
>> There is also this RFC from Sven, which tries to inline some of the
>> generic system call functions, in order to avoid function calls:
>> https://lore.kernel.org/lkml/20230516133810.171487-1-svens@linux.ibm.com/
>> 
>> I stumbled upon CONFIG_INIT_STACK_NONE only by accident when wondering why
>> the compiler would generate quite some instructions which aren't necessary,
>> just to zero variables. For the getpid() system call this makes a runtime
>> difference of ~3%, which is quite a bit.
>
> Hm, that does seem high. It implies there are large variable that are
> being passed by reference, perhaps in the syscall path? I had similar
> problems a while back on x86 but due to stack-protector seeing the
> register arrays and thinking they needed protection. I had to explicitly
> turn that off for the entry code, since they're provably safe. :)

From looking at our s390 specific entry code i don't see big arrays on
the stack, but let me do some profiling. Maybe i missed something.

Regards,
Sven

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-05-30  6:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-25 18:40 s390/defconfigs: set CONFIG_INIT_STACK_NONE=y Kees Cook
2023-05-26 13:42 ` Heiko Carstens
2023-05-26 16:37   ` Kees Cook
2023-05-30  6:44     ` Sven Schnelle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).