Re: [Buildroot] [PATCH] package/glibc: optional MTE support in glibc

From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: "irgstg@gmail.com" <irgstg@gmail.com>
Cc: Bernd Kuhls <bernd.kuhls@t-online.de>,
	Romain Naour <romain.naour@gmail.com>,
	buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH] package/glibc: optional MTE support in glibc
Date: Sun, 31 Oct 2021 18:52:36 +0100	[thread overview]
Message-ID: <20211031185236.25e4b639@windsurf> (raw)
In-Reply-To: <20211030083753.192-1-irgstg@gmail.com>

Hello,

Thanks a lot for your contribution! See below some comments.

On Sat, 30 Oct 2021 11:37:50 +0300
"irgstg@gmail.com" <irgstg@gmail.com> wrote:

> Adding option BR2_PACKAGE_GLIBC_MTE to configure glibc with MTE support.
> When enabled, glibcs configuration process runs with `--enable-memory-tagging` [1].
> 
> MTE (Memory Tagging Extension) is an ISA extension which aims to improve security on ARMv8.5-A [2].
> The linux kernel supports this feature via CONFIG_ARM64_MTE.
> In order for glibc's MTE support to work properly, the running kernel has to support that.

Could you wrap the lines of your commit log to 72 columns?

> dependencies:
>  - supported on aarch64 only (BR2_aarch64)
>  - requires binutils version 2.33.1 or newer (!B2_BINUTILS_VERSION_2_32_X)
>  - requires at least linux 5.4 headers (BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_4)
> 
> [1] https://sourceware.org/git/?p=glibc.git;a=blob;f=INSTALL;h=02dcf6b1ca3a4c43a17fdcae5e7dae8189c1c50b;hb=HEAD#l145
> [2] https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/enhancing-memory-safety
> 
> Signed-off-by: irgstg@gmail.com <irgstg@gmail.com>

Could you resend with a real name ? We cannot accept anonymous
contributions.

> +config BR2_PACKAGE_GLIBC_MTE
> +	bool "Install glibc support for MTE"

Repeating that it's glibc related is not necessary, and making "MTE"
explicit seems useful. So perhaps:

	bool "enable Memory Tagging Extension (MTE) support"

> +	depends on BR2_aarch64
> +	depends on !BR2_BINUTILS_VERSION_2_32_X
> +	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_4
> +	help
> +	  Enabling this option will configure, compile
> +	  and install glibc support for MTE (Memory Tagging Extension).
> +	  Please be aware, in order for that to work properly,
> +	  kernel support for MTE (CONFIG_ARM64_MTE), and a
> +	  supported CPU are needed.

Instead of just saying so, you could do this in glibc.mk:

ifeq ($(BR2_PACKAGE_GLIBC_MTE),y)
GLIBC_CONF_OPTS += --enable-memory-tagging
define GLIBC_LINUX_CONFIG_FIXUPS
	$(call KCONFIG_ENABLE_OPT,CONFIG_ARM64_MTE)
endef
endif

Of course, you need to add $(GLIBC_CONF_OPTS) inside
GLIBC_CONFIGURE_CMDS.

Other than that, it looks good to me.

Do we need special compiler options to make use of MTE ?

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot