2023-06-22 17:32:27

by Ammar Faizi

[permalink] [raw]
Subject: [RFC PATCH liburing v1 3/3] src/Makefile: Allow using stack protector with libc

Currently, the stack protector is forcefully disabled. Let's allow using
the stack protector feature only if libc is used.

The stack protector will remain disabled by default if no custom CFLAGS
are provided. This ensures the default behavior doesn't change while
still offering the option to enable the stack protector.

Cc: Stefan Hajnoczi <[email protected]>
Cc: Guillem Jover <[email protected]>
Co-authored-by: Alviro Iskandar Setiawan <[email protected]>
Signed-off-by: Alviro Iskandar Setiawan <[email protected]>
Signed-off-by: Ammar Faizi <[email protected]>
---
src/Makefile | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/Makefile b/src/Makefile
index 951c48fc6797be75..c4c28cbe87c7a8de 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -10,9 +10,8 @@ CPPFLAGS ?=
override CPPFLAGS += -D_GNU_SOURCE \
-Iinclude/ -include ../config-host.h \
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
-CFLAGS ?= -g -O3 -Wall -Wextra
+CFLAGS ?= -g -O3 -Wall -Wextra -fno-stack-protector
override CFLAGS += -Wno-unused-parameter \
- -fno-stack-protector \
-DLIBURING_INTERNAL \
$(LIBURING_CFLAGS)
SO_CFLAGS=-fPIC $(CFLAGS)
@@ -46,8 +45,8 @@ liburing_srcs := setup.c queue.c register.c syscall.c version.c

ifeq ($(CONFIG_NOLIBC),y)
liburing_srcs += nolibc.c
- override CFLAGS += -nostdlib -nodefaultlibs -ffreestanding -fno-builtin
- override CPPFLAGS += -nostdlib -nodefaultlibs -ffreestanding -fno-builtin
+ override CFLAGS += -nostdlib -nodefaultlibs -ffreestanding -fno-builtin -fno-stack-protector
+ override CPPFLAGS += -nostdlib -nodefaultlibs -ffreestanding -fno-builtin -fno-stack-protector
override LINK_FLAGS += -nostdlib -nodefaultlibs
endif

--
Ammar Faizi



2023-06-22 18:12:08

by Thomas Weißschuh

[permalink] [raw]
Subject: Re: [RFC PATCH liburing v1 3/3] src/Makefile: Allow using stack protector with libc

On 2023-06-23 00:20:29+0700, Ammar Faizi wrote:
> Currently, the stack protector is forcefully disabled. Let's allow using
> the stack protector feature only if libc is used.
>
> The stack protector will remain disabled by default if no custom CFLAGS
> are provided. This ensures the default behavior doesn't change while
> still offering the option to enable the stack protector.

FYI

There are patches in the pipeline that enable stackprotector support for
nolibc [0]. They should land in 6.5.

It only supports "global" mode and not per-thread-data.
But as nolibc does not support threads anyways that should not matter.
A compiler flag has to be passed though, but that can be automated [1].

So the -fno-stack-protector can probably be removed completely.

[0] https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/tree/tools/include/nolibc/stackprotector.h?h=dev.2023.06.16a
[1] https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/tree/tools/testing/selftests/nolibc/Makefile?h=dev.2023.06.16a#n81

> Cc: Stefan Hajnoczi <[email protected]>
> Cc: Guillem Jover <[email protected]>
> Co-authored-by: Alviro Iskandar Setiawan <[email protected]>
> Signed-off-by: Alviro Iskandar Setiawan <[email protected]>
> Signed-off-by: Ammar Faizi <[email protected]>
> ---
> src/Makefile | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/src/Makefile b/src/Makefile
> index 951c48fc6797be75..c4c28cbe87c7a8de 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -10,9 +10,8 @@ CPPFLAGS ?=
> override CPPFLAGS += -D_GNU_SOURCE \
> -Iinclude/ -include ../config-host.h \
> -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
> -CFLAGS ?= -g -O3 -Wall -Wextra
> +CFLAGS ?= -g -O3 -Wall -Wextra -fno-stack-protector
> override CFLAGS += -Wno-unused-parameter \
> - -fno-stack-protector \
> -DLIBURING_INTERNAL \
> $(LIBURING_CFLAGS)
> SO_CFLAGS=-fPIC $(CFLAGS)
> @@ -46,8 +45,8 @@ liburing_srcs := setup.c queue.c register.c syscall.c version.c
>
> ifeq ($(CONFIG_NOLIBC),y)
> liburing_srcs += nolibc.c
> - override CFLAGS += -nostdlib -nodefaultlibs -ffreestanding -fno-builtin
> - override CPPFLAGS += -nostdlib -nodefaultlibs -ffreestanding -fno-builtin
> + override CFLAGS += -nostdlib -nodefaultlibs -ffreestanding -fno-builtin -fno-stack-protector
> + override CPPFLAGS += -nostdlib -nodefaultlibs -ffreestanding -fno-builtin -fno-stack-protector
> override LINK_FLAGS += -nostdlib -nodefaultlibs
> endif
>
> --
> Ammar Faizi
>

2023-06-22 23:05:21

by Ammar Faizi

[permalink] [raw]
Subject: Re: [RFC PATCH liburing v1 3/3] src/Makefile: Allow using stack protector with libc

On Thu, Jun 22, 2023 at 07:57:38PM +0200, Thomas Wei?schuh wrote:
> There are patches in the pipeline that enable stackprotector support for
> nolibc [0]. They should land in 6.5.

That's interesting. I haven't been following Willy's tree for a while.
Hope 6.4 stable goes well by the end of this week.

> It only supports "global" mode and not per-thread-data.
> But as nolibc does not support threads anyways that should not matter.
> A compiler flag has to be passed though, but that can be automated [1].
>
> So the -fno-stack-protector can probably be removed completely.
>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/tree/tools/include/nolibc/stackprotector.h?h=dev.2023.06.16a
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/tree/tools/testing/selftests/nolibc/Makefile?h=dev.2023.06.16a#n81

This is a bit problematic because liburing.so and liburing.a must also
be compatible with apps that use libc. Note that liburing nolibc is also
used by apps that use libc.

The problem when an app uses libc.so and liburing.a:

Stack-protector functions from liburing nolibc will override the
stack-protector functions from libc because statically linked functions
will take precedence. The end result, the app will always use the
"global" mode stack protector even if it's multithreaded. There may be a
way to make those functions private to liburing only, but I don't know.

We had a similar problem with memset() in liburing:

https://git.kernel.org/pub/scm/linux/kernel/git/axboe/liburing.git/commit/?id=db5403e58083bef48d72656d7dea53a9f7affef4

Also, the app has to be compiled with those specific flags, which is out
of our control. Plus, I wonder if there is a chance to call
__stack_chk_init() from a static library point of view where we don't
control the entry point (__start).

Therefore, I won't implement the stack protector for liburing under
CONFIG_NOLIBC enabled. So far, I see that using the stack protector for
liburing nolibc is more trouble than it's worth.

But anyway, it's nice to see your stack protector work.

Regards,
--
Ammar Faizi