| 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
| ||
|
Message-ID: <CAFULd4b8de+uEtwYBNNip9raVp1y-CcxO3BX7NrWDVHHVSmsnQ@mail.gmail.com>
Date: Wed, 21 Jan 2026 11:11:15 +0100
From: Uros Bizjak <ubizjak@...il.com>
To: "H. Peter Anvin" <hpa@...or.com>
Cc: Thomas Gleixner <tglx@...nel.org>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, Petr Mladek <pmladek@...e.com>,
Andrew Morton <akpm@...ux-foundation.org>, Kees Cook <kees@...nel.org>,
"Peter Zijlstra (Intel)" <peterz@...radead.org>, Nathan Chancellor <nathan@...nel.org>,
Kiryl Shutsemau <kas@...nel.org>, Rick Edgecombe <rick.p.edgecombe@...el.com>,
linux-kernel@...r.kernel.org, linux-coco@...ts.linux.dev, x86@...nel.org
Subject: Re: [PATCH v1 13/14] x86/boot: simplify x86/boot/cmdline.c by using __seg_fs
On Tue, Jan 20, 2026 at 8:54 PM H. Peter Anvin <hpa@...or.com> wrote:
>
> arch/x86/boot/cmdline.c is compiled in 16, 32, or 64-bit mode. In the
> 16-bit case it needs to access an out-of-data-segment pointer.
>
> This has been handled in the past by using a *really* ugly wrapper in
> the 32/64-bit code combined with an inline function in the 16-bit code.
>
> Using __seg_fs for the real-mode case allows for a much cleaner way of
> handling this difference. Futhermore, moving the code for typing and
> obtaining the pointer into cmdline.c itself avoids having to create
> and push an extra argument, which is particularly inefficient in the
> real-mode code as it requires a range check and pushes the argument
> count past 3. Furthermore, co-locating this code with the user makes it
> a lot clearer what the constraints are on this code, even though it
> means using #ifdef _SETUP.
>
> Instead of limit-checking the command line to the real-mode segment
> limit, limit-check it to COMMAND_LINE_SIZE (which is always less than
> 64K, and thus automatically incorporates the real-mode segment limit
> check.)
>
> If compiling for 32 bits, trying to add ext_cmd_line_ptr in
> get_cmd_line_ptr() is futile as unsigned long, and pointers, are only
> 32 bits wide. Instead, limit-check the command line pointer and fail
> if it is >= 4 GiB, just as the real-mode code fails if the pointer is
> >= 1 MiB.
>
> Since the kaslr code depends on get_cmd_line_ptr(), retain it as a
> global function.
>
> Signed-off-by: H. Peter Anvin (Intel) <hpa@...or.com>
Acked-by: Uros Bizjak <ubizjak@...il.com>
> ---
> arch/x86/boot/boot.h | 23 +--------
> arch/x86/boot/cmdline.c | 79 +++++++++++++++++++++++-------
> arch/x86/boot/compressed/cmdline.c | 29 -----------
> 3 files changed, 64 insertions(+), 67 deletions(-)
>
> diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
> index 584c89d0738b..8512db8b3f8e 100644
> --- a/arch/x86/boot/boot.h
> +++ b/arch/x86/boot/boot.h
> @@ -216,27 +216,8 @@ struct biosregs {
> void intcall(u8 int_no, const struct biosregs *ireg, struct biosregs *oreg);
>
> /* cmdline.c */
> -int __cmdline_find_option(unsigned long cmdline_ptr, const char *option, char *buffer, int bufsize);
> -int __cmdline_find_option_bool(unsigned long cmdline_ptr, const char *option);
> -static inline int cmdline_find_option(const char *option, char *buffer, int bufsize)
> -{
> - unsigned long cmd_line_ptr = boot_params.hdr.cmd_line_ptr;
> -
> - if (cmd_line_ptr >= 0x100000)
> - return -1; /* inaccessible */
> -
> - return __cmdline_find_option(cmd_line_ptr, option, buffer, bufsize);
> -}
> -
> -static inline int cmdline_find_option_bool(const char *option)
> -{
> - unsigned long cmd_line_ptr = boot_params.hdr.cmd_line_ptr;
> -
> - if (cmd_line_ptr >= 0x100000)
> - return -1; /* inaccessible */
> -
> - return __cmdline_find_option_bool(cmd_line_ptr, option);
> -}
> +int cmdline_find_option(const char *option, char *buffer, int bufsize);
> +int cmdline_find_option_bool(const char *option);
>
> /* cpu.c, cpucheck.c */
> int check_cpu(int *cpu_level_ptr, int *req_level_ptr, u32 **err_flags_ptr);
> diff --git a/arch/x86/boot/cmdline.c b/arch/x86/boot/cmdline.c
> index 21d56ae83cdf..3f31fcbed673 100644
> --- a/arch/x86/boot/cmdline.c
> +++ b/arch/x86/boot/cmdline.c
> @@ -11,12 +11,59 @@
> */
>
> #include "boot.h"
> +#include <asm/setup.h>
> +#include <asm/bootparam.h>
>
> static inline int myisspace(u8 c)
> {
> return c <= ' '; /* Close enough approximation */
> }
>
> +#ifdef _SETUP
> +typedef const char __seg_fs *cptr_t;
> +static inline cptr_t get_cptr(void)
> +{
> + /*
> + * Note: there is no reason to check ext_cmd_line_ptr here,
> + * because it falls outside of boot_params.hdr and therefore
> + * will always be zero when entering through the real-mode
> + * entry point.
> + */
> + unsigned long ptr = boot_params.hdr.cmd_line_ptr;
> +
> + /*
> + * The -16 here serves two purposes:
> + * 1. It means the segbase >= 0x100000 check also doubles as
> + * a check for the command line pointer being zero.
> + * 2. It means this routine won't return a NULL pointer for
> + * a valid address; it will always return a pointer in the
> + * range 0x10-0x1f inclusive.
> + */
> + unsigned long segbase = (ptr - 16) & ~15;
> + if (segbase >= 0x100000)
> + return NULL;
> +
> + set_fs(segbase >> 4);
> + return (cptr_t)(ptr - segbase);
> +}
> +#else
> +unsigned long get_cmd_line_ptr(void)
> +{
> + unsigned long ptr = boot_params_ptr->hdr.cmd_line_ptr;
> + if (sizeof(unsigned long) > 4)
> + ptr += (u64)boot_params_ptr->ext_cmd_line_ptr << 32;
> + else if (boot_params_ptr->ext_cmd_line_ptr)
> + return 0; /* Inaccessible due to pointer overflow */
> +
> + return ptr;
> +}
> +typedef const char *cptr_t;
> +static inline cptr_t get_cptr(void)
> +{
> + return (cptr_t)get_cmd_line_ptr();
> +}
> +#endif
> +
> /*
> * Find a non-boolean option, that is, "option=argument". In accordance
> * with standard Linux practice, if this option is repeated, this returns
> @@ -25,9 +72,9 @@ static inline int myisspace(u8 c)
> * Returns the length of the argument (regardless of if it was
> * truncated to fit in the buffer), or -1 on not found.
> */
> -int __cmdline_find_option(unsigned long cmdline_ptr, const char *option, char *buffer, int bufsize)
> +int cmdline_find_option(const char *option, char *buffer, int bufsize)
> {
> - addr_t cptr;
> + cptr_t cptr, eptr;
> char c;
> int len = -1;
> const char *opptr = NULL;
> @@ -39,13 +86,12 @@ int __cmdline_find_option(unsigned long cmdline_ptr, const char *option, char *b
> st_bufcpy /* Copying this to buffer */
> } state = st_wordstart;
>
> - if (!cmdline_ptr)
> - return -1; /* No command line */
> + cptr = get_cptr();
> + if (!cptr)
> + return -1; /* No command line or invalid pointer */
> + eptr = cptr + COMMAND_LINE_SIZE - 1;
>
> - cptr = cmdline_ptr & 0xf;
> - set_fs(cmdline_ptr >> 4);
> -
> - while (cptr < 0x10000 && (c = rdfs8(cptr++))) {
> + while (cptr < eptr && (c = *cptr++)) {
> switch (state) {
> case st_wordstart:
> if (myisspace(c))
> @@ -97,9 +143,9 @@ int __cmdline_find_option(unsigned long cmdline_ptr, const char *option, char *b
> * Returns the position of that option (starts counting with 1)
> * or 0 on not found
> */
> -int __cmdline_find_option_bool(unsigned long cmdline_ptr, const char *option)
> +int cmdline_find_option_bool(const char *option)
> {
> - addr_t cptr;
> + cptr_t cptr, eptr;
> char c;
> int pos = 0, wstart = 0;
> const char *opptr = NULL;
> @@ -109,14 +155,13 @@ int __cmdline_find_option_bool(unsigned long cmdline_ptr, const char *option)
> st_wordskip, /* Miscompare, skip */
> } state = st_wordstart;
>
> - if (!cmdline_ptr)
> - return -1; /* No command line */
> -
> - cptr = cmdline_ptr & 0xf;
> - set_fs(cmdline_ptr >> 4);
> + cptr = get_cptr();
> + if (!cptr)
> + return -1; /* No command line or invalid pointer */
> + eptr = cptr + COMMAND_LINE_SIZE - 1;
>
> - while (cptr < 0x10000) {
> - c = rdfs8(cptr++);
> + while (cptr <= eptr) {
> + c = *cptr++;
> pos++;
>
> switch (state) {
> diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c
> index e162d7f59cc5..0d9267a21012 100644
> --- a/arch/x86/boot/compressed/cmdline.c
> +++ b/arch/x86/boot/compressed/cmdline.c
> @@ -1,32 +1,3 @@
> // SPDX-License-Identifier: GPL-2.0
> #include "misc.h"
> -
> -#include <asm/bootparam.h>
> -
> -static unsigned long fs;
> -static inline void set_fs(unsigned long seg)
> -{
> - fs = seg << 4; /* shift it back */
> -}
> -typedef unsigned long addr_t;
> -static inline char rdfs8(addr_t addr)
> -{
> - return *((char *)(fs + addr));
> -}
> #include "../cmdline.c"
> -unsigned long get_cmd_line_ptr(void)
> -{
> - unsigned long cmd_line_ptr = boot_params_ptr->hdr.cmd_line_ptr;
> -
> - cmd_line_ptr |= (u64)boot_params_ptr->ext_cmd_line_ptr << 32;
> -
> - return cmd_line_ptr;
> -}
> -int cmdline_find_option(const char *option, char *buffer, int bufsize)
> -{
> - return __cmdline_find_option(get_cmd_line_ptr(), option, buffer, bufsize);
> -}
> -int cmdline_find_option_bool(const char *option)
> -{
> - return __cmdline_find_option_bool(get_cmd_line_ptr(), option);
> -}
> --
> 2.52.0
>
Powered by blists - more mailing lists