| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 12 Dec 2019 14:29:40 +0000
From: Matthias Maennich <maennich@...gle.com>
To: Jessica Yu <jeyu@...nel.org>
Cc: linux-kernel@...r.kernel.org,
Masahiro Yamada <yamada.masahiro@...ionext.com>,
Rasmus Villemoes <linux@...musvillemoes.dk>,
Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH v4] export.h: reduce __ksymtab_strings string duplication
by using "MS" section flags
On Thu, Dec 12, 2019 at 03:16:13PM +0100, Jessica Yu wrote:
>Commit c3a6cf19e695 ("export: avoid code duplication in
>include/linux/export.h") refactors export.h quite nicely, but introduces
>a slight increase in memory usage due to using the empty string ""
>instead of NULL to indicate that an exported symbol has no namespace. As
>mentioned in that commit, this meant an increase of 1 byte per exported
>symbol without a namespace. For example, if a kernel configuration has
>about 10k exported symbols, this would mean that the size of
>__ksymtab_strings would increase by roughly 10kB.
>
>We can alleviate this situation by utilizing the SHF_MERGE and
>SHF_STRING section flags. SHF_MERGE|SHF_STRING indicate to the linker
>that the data in the section are null-terminated strings that can be
>merged to eliminate duplication. More specifically, from the binutils
>documentation - "for sections with both M and S, a string which is a
>suffix of a larger string is considered a duplicate. Thus "def" will be
>merged with "abcdef"; A reference to the first "def" will be changed to
>a reference to "abcdef"+3". Thus, all the empty strings would be merged
>as well as any strings that can be merged according to the cited method
>above. For example, "memset" and "__memset" would be merged to just
>"__memset" in __ksymtab_strings.
>
>As of v5.4-rc5, the following statistics were gathered with x86
>defconfig with approximately 10.7k exported symbols.
>
>Size of __ksymtab_strings in vmlinux:
>-------------------------------------
>v5.4-rc5: 213834 bytes
>v5.4-rc5 with commit c3a6cf19e695: 224455 bytes
>v5.4-rc5 with this patch: 205759 bytes
>
>So, we already see memory savings of ~8kB compared to vanilla -rc5 and
>savings of nearly 18.7kB compared to -rc5 with commit c3a6cf19e695 on top.
>
>Unfortunately, as of this writing, strings will not get deduplicated for
>kernel modules, as ld does not do the deduplication for
>SHF_MERGE|SHF_STRINGS sections for relocatable files (ld -r), which
>kernel modules are. A patch for ld is currently being worked on to
>hopefully allow for string deduplication in relocatable files in the
>future.
>
>Suggested-by: Rasmus Villemoes <linux@...musvillemoes.dk>
>Reviewed-by: Masahiro Yamada <masahiroy@...nel.org>
>Signed-off-by: Jessica Yu <jeyu@...nel.org>
>---
>v4:
> - fix the comment above ___EXPORT_SYMBOL to be more specific about what
> entries are being placed in their respective sections.
>
> include/asm-generic/export.h | 8 +++++---
> include/linux/export.h | 27 ++++++++++++++++++++-------
>
> 2 files changed, 25 insertions(+), 10 deletions(-)
>
>diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
>index afddc5442e92..365345f9a9e3 100644
>--- a/include/asm-generic/export.h
>+++ b/include/asm-generic/export.h
>@@ -27,9 +27,11 @@
> .endm
>
> /*
>- * note on .section use: @progbits vs %progbits nastiness doesn't matter,
>- * since we immediately emit into those sections anyway.
>+ * note on .section use: we specify progbits since usage of the "M" (SHF_MERGE)
>+ * section flag requires it. Use '%progbits' instead of '@...gbits' since the
>+ * former apparently works on all arches according to the binutils source.
> */
>+
> .macro ___EXPORT_SYMBOL name,val,sec
> #ifdef CONFIG_MODULES
> .section ___ksymtab\sec+\name,"a"
>@@ -37,7 +39,7 @@
> __ksymtab_\name:
> __put \val, __kstrtab_\name
> .previous
>- .section __ksymtab_strings,"a"
>+ .section __ksymtab_strings,"aMS",%progbits,1
> __kstrtab_\name:
> .asciz "\name"
> .previous
>diff --git a/include/linux/export.h b/include/linux/export.h
>index 627841448293..c166d35e3d76 100644
>--- a/include/linux/export.h
>+++ b/include/linux/export.h
>@@ -82,16 +82,29 @@ struct kernel_symbol {
>
> #else
>
>-/* For every exported symbol, place a struct in the __ksymtab section */
>+/*
>+ * For every exported symbol, do the following:
>+ *
>+ * - If applicable, place a CRC entry in the __kcrctab section.
>+ * - Put the name of the symbol and namespace (empty string "" for none) in
>+ * __ksymtab_strings.
>+ * - Place a struct kernel_symbol entry in the __ksymtab section.
>+ *
>+ * note on .section use: we specify progbits since usage of the "M" (SHF_MERGE)
>+ * section flag requires it. Use '%progbits' instead of '@...gbits' since the
>+ * former apparently works on all arches according to the binutils source.
>+ */
> #define ___EXPORT_SYMBOL(sym, sec, ns) \
> extern typeof(sym) sym; \
>+ extern const char __kstrtab_##sym[]; \
>+ extern const char __kstrtabns_##sym[]; \
> __CRC_SYMBOL(sym, sec); \
>- static const char __kstrtab_##sym[] \
>- __attribute__((section("__ksymtab_strings"), used, aligned(1))) \
>- = #sym; \
>- static const char __kstrtabns_##sym[] \
>- __attribute__((section("__ksymtab_strings"), used, aligned(1))) \
>- = ns; \
>+ asm(" .section \"__ksymtab_strings\",\"aMS\",%progbits,1\n" \
>+ "__kstrtab_" #sym ": \n" \
>+ " .asciz \"" #sym "\" \n" \
>+ "__kstrtabns_" #sym ": \n" \
>+ " .asciz \"" ns "\" \n" \
>+ " .previous \n"); \
nit: You might want to align the newline characters up to the asm line.
Thanks for working on this!
Reviewed-by: Matthias Maennich <maennich@...gle.com>
Cheers,
Matthias
> __KSYMTAB_ENTRY(sym, sec)
>
> #endif
>--
>2.16.4
>
Powered by blists - more mailing lists