| 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: <20250207181258.233674df@pumpkin>
Date: Fri, 7 Feb 2025 18:12:58 +0000
From: David Laight <david.laight.linux@...il.com>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: linux-kernel@...r.kernel.org, Alice Ryhl <aliceryhl@...gle.com>, Boqun
Feng <boqun.feng@...il.com>, rust-for-linux@...r.kernel.org,
netdev@...r.kernel.org, andrew@...n.ch, hkallweit1@...il.com,
tmgross@...ch.edu, ojeda@...nel.org, alex.gaynor@...il.com,
gary@...yguo.net, bjorn3_gh@...tonmail.com, benno.lossin@...ton.me,
a.hindborg@...sung.com, anna-maria@...utronix.de, frederic@...nel.org,
tglx@...utronix.de, arnd@...db.de, jstultz@...gle.com, sboyd@...nel.org,
mingo@...hat.com, peterz@...radead.org, juri.lelli@...hat.com,
vincent.guittot@...aro.org, dietmar.eggemann@....com, rostedt@...dmis.org,
bsegall@...gle.com, mgorman@...e.de, vschneid@...hat.com,
tgunders@...hat.com, me@...enk.dev
Subject: Re: [PATCH v10 1/8] sched/core: Add __might_sleep_precision()
On Fri, 7 Feb 2025 22:26:16 +0900
FUJITA Tomonori <fujita.tomonori@...il.com> wrote:
> Add __might_sleep_precision(), Rust friendly version of
> __might_sleep(), which takes a pointer to a string with the length
> instead of a null-terminated string.
>
> Rust's core::panic::Location::file(), which gives the file name of a
> caller, doesn't provide a null-terminated
> string. __might_sleep_precision() uses a precision specifier in the
> printk format, which specifies the length of a string; a string
> doesn't need to be a null-terminated.
>
> Modify __might_sleep() to call __might_sleep_precision() but the
> impact should be negligible. strlen() isn't called in a normal case;
> it's called only when printing the error (sleeping function called
> from invalid context).
>
> Note that Location::file() providing a null-terminated string for
> better C interoperability is under discussion [1].
>
> Link: https://github.com/rust-lang/libs-team/issues/466 [1]
> Reviewed-by: Alice Ryhl <aliceryhl@...gle.com>
> Co-developed-by: Boqun Feng <boqun.feng@...il.com>
> Signed-off-by: Boqun Feng <boqun.feng@...il.com>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@...il.com>
> ---
> include/linux/kernel.h | 2 ++
> kernel/sched/core.c | 55 ++++++++++++++++++++++++++----------------
> 2 files changed, 36 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index be2e8c0a187e..086ee1dc447e 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -87,6 +87,7 @@ extern int dynamic_might_resched(void);
> #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
> extern void __might_resched(const char *file, int line, unsigned int offsets);
> extern void __might_sleep(const char *file, int line);
> +extern void __might_sleep_precision(const char *file, int len, int line);
> extern void __cant_sleep(const char *file, int line, int preempt_offset);
> extern void __cant_migrate(const char *file, int line);
>
> @@ -145,6 +146,7 @@ extern void __cant_migrate(const char *file, int line);
> static inline void __might_resched(const char *file, int line,
> unsigned int offsets) { }
> static inline void __might_sleep(const char *file, int line) { }
> +static inline void __might_sleep_precision(const char *file, int len, int line) { }
> # define might_sleep() do { might_resched(); } while (0)
> # define cant_sleep() do { } while (0)
> # define cant_migrate() do { } while (0)
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 165c90ba64ea..d308f2a8692e 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -8678,24 +8678,6 @@ void __init sched_init(void)
>
> #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
>
> -void __might_sleep(const char *file, int line)
> -{
> - unsigned int state = get_current_state();
> - /*
> - * Blocking primitives will set (and therefore destroy) current->state,
> - * since we will exit with TASK_RUNNING make sure we enter with it,
> - * otherwise we will destroy state.
> - */
> - WARN_ONCE(state != TASK_RUNNING && current->task_state_change,
> - "do not call blocking ops when !TASK_RUNNING; "
> - "state=%x set at [<%p>] %pS\n", state,
> - (void *)current->task_state_change,
> - (void *)current->task_state_change);
> -
> - __might_resched(file, line, 0);
> -}
> -EXPORT_SYMBOL(__might_sleep);
> -
> static void print_preempt_disable_ip(int preempt_offset, unsigned long ip)
> {
> if (!IS_ENABLED(CONFIG_DEBUG_PREEMPT))
> @@ -8717,7 +8699,8 @@ static inline bool resched_offsets_ok(unsigned int offsets)
> return nested == offsets;
> }
>
> -void __might_resched(const char *file, int line, unsigned int offsets)
> +static void __might_resched_precision(const char *file, int len, int line,
For clarity that ought to be file_len.
> + unsigned int offsets)
> {
> /* Ratelimiting timestamp: */
> static unsigned long prev_jiffy;
> @@ -8740,8 +8723,10 @@ void __might_resched(const char *file, int line, unsigned int offsets)
> /* Save this before calling printk(), since that will clobber it: */
> preempt_disable_ip = get_preempt_disable_ip(current);
>
> - pr_err("BUG: sleeping function called from invalid context at %s:%d\n",
> - file, line);
> + if (len < 0)
> + len = strlen(file);
No need for strlen(), just use a big number instead of -1.
Anything bigger than a sane upper limit on the filename length will do.
David
> + pr_err("BUG: sleeping function called from invalid context at %.*s:%d\n",
> + len, file, line);
> pr_err("in_atomic(): %d, irqs_disabled(): %d, non_block: %d, pid: %d, name: %s\n",
> in_atomic(), irqs_disabled(), current->non_block_count,
> current->pid, current->comm);
> @@ -8766,8 +8751,36 @@ void __might_resched(const char *file, int line, unsigned int offsets)
> dump_stack();
> add_taint(TAINT_WARN, LOCKDEP_STILL_OK);
> }
> +
> +void __might_resched(const char *file, int line, unsigned int offsets)
> +{
> + __might_resched_precision(file, -1, line, offsets);
> +}
> EXPORT_SYMBOL(__might_resched);
>
> +void __might_sleep_precision(const char *file, int len, int line)
> +{
> + unsigned int state = get_current_state();
> + /*
> + * Blocking primitives will set (and therefore destroy) current->state,
> + * since we will exit with TASK_RUNNING make sure we enter with it,
> + * otherwise we will destroy state.
> + */
> + WARN_ONCE(state != TASK_RUNNING && current->task_state_change,
> + "do not call blocking ops when !TASK_RUNNING; "
> + "state=%x set at [<%p>] %pS\n", state,
> + (void *)current->task_state_change,
> + (void *)current->task_state_change);
> +
> + __might_resched_precision(file, len, line, 0);
> +}
> +
> +void __might_sleep(const char *file, int line)
> +{
> + __might_sleep_precision(file, -1, line);
> +}
> +EXPORT_SYMBOL(__might_sleep);
> +
> void __cant_sleep(const char *file, int line, int preempt_offset)
> {
> static unsigned long prev_jiffy;
Powered by blists - more mailing lists