linux-kernel - Re: [PATCH 0/3] syscalls: clean up stub naming convention

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 9 Apr 2018 08:49:45 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Dominik Brodowski <linux@...inikbrodowski.net>
Cc:     linux-kernel@...r.kernel.org, Al Viro <viro@...iv.linux.org.uk>,
        Andi Kleen <ak@...ux.intel.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andy Lutomirski <luto@...nel.org>,
        Brian Gerst <brgerst@...il.com>,
        Denys Vlasenko <dvlasenk@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>, x86@...nel.org,
        Maninder Singh <maninder1.s@...sung.com>,
        Arnd Bergmann <arnd@...db.de>,
        linux-arch <linux-arch@...r.kernel.org>
Subject: Re: [PATCH 0/3] syscalls: clean up stub naming convention


* Dominik Brodowski <linux@...inikbrodowski.net> wrote:

> +emit_stub() {
> +    entry="$1"
> +    if [ "${entry}" != "${entry#__ia32_sys_}" ]; then
> +	# We need a stub named __ia32_sys which is common to 64-bit
> +	# except for a different pt_regs layout.
> +	stubname=${entry#__ia32_sys_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __IA32_SYS_STUBx"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +    elif [ "$entry" != "${entry#__x32_compat_sys}" ]; then
> +	# We need a stub named __x32_compat_sys_ which decodes a
> +	# 64-bit pt_regs and then calls the real syscall function 
> +	stubname="${entry%%/*}" # handle qualifier
> +	stubname=${stubname#__x32_compat_sys_} # handle prefix
> +	echo "#define __X32_COMPAT_SYS_STUBx_${stubname} __X32_COMPAT_SYS_STUBx"
> +	echo "#define ASM_X86_HAS__x32_compat_sys_${stubname} 1"
> +    elif [ "$entry" != "${entry#__ia32_compat_sys_x86}" ]; then
> +	# The compat entry starts with __ia32_compat_sys_x86, so it
> +	# is a specific x86 compat syscall; no need for __ia32_sys_*()
> +	stubname=${entry#__ia32_compat_sys_x86_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +    elif [ "$entry" != "${entry#__ia32_compat_sys_}" ]; then
> +	# The compat entry starts with __ia32_compat_sys, so it is
> +	# is a generic x86 compat syscall; no need for __ia32_sys_*()
> +	stubname=${entry#__ia32_compat_sys_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +    fi;
> +}

I only have a bikeshed painting comment, even in shell scripts please try to 
follow the kernel comment style visually, i.e. something like:

> +    if [ "${entry}" != "${entry#__ia32_sys_}" ]; then
> +
> +	#
> +	# We need a stub named __ia32_sys which is common to 64-bit
> +	# except for a different pt_regs layout.
> +	#
> +	stubname=${entry#__ia32_sys_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __IA32_SYS_STUBx"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +
> +    elif [ "$entry" != "${entry#__x32_compat_sys}" ]; then
> +
> +	#
> +	# We need a stub named __x32_compat_sys_ which decodes a
> +	# 64-bit pt_regs and then calls the real syscall function 
> +     #
> +	stubname="${entry%%/*}" # handle qualifier
> +	stubname=${stubname#__x32_compat_sys_} # handle prefix
> +	echo "#define __X32_COMPAT_SYS_STUBx_${stubname} __X32_COMPAT_SYS_STUBx"
> +	echo "#define ASM_X86_HAS__x32_compat_sys_${stubname} 1"
> +
> +    elif [ "$entry" != "${entry#__ia32_compat_sys_x86}" ]; then
> +
> +     #
> +	# The compat entry starts with __ia32_compat_sys_x86, so it
> +	# is a specific x86 compat syscall; no need for __ia32_sys_*()
> +     #
> +	stubname=${entry#__ia32_compat_sys_x86_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +
> +    elif [ "$entry" != "${entry#__ia32_compat_sys_}" ]; then
> +
> +     #
> +	# The compat entry starts with __ia32_compat_sys, so it is
> +	# is a generic x86 compat syscall; no need for __ia32_sys_*()
> +     #
> +	stubname=${entry#__ia32_compat_sys_}
> +	echo "#define __IA32_SYS_STUBx_${stubname} __SYSCALL_STUBx_UNUSED"
> +	echo "#define ASM_X86_HAS__ia32_sys_${stubname} 1"
> +
> +    fi;

( Also note the vertical separation that helps see the overall structure a bit 
  better. )

But more fundamentally, that's an awful lot of complex scripting to save 4K 
(unused) kernel text, not sure it's worth it.

Once we grow proper LTO support I'd guess these unused functions would go away 
naturally? None should have a __visible annotation.

Thanks,

	Ingo