Re: [PATCH v3 3/4] uaccess: Check no rescheduling function is called in unsafe region

Valentin Schneider
2019-01-30T16:58:09Z

On 15/01/2019 13:58, Julien Thierry wrote:
[...]> @@ -6151,6 +6159,20 @@ void ___might_sleep(const char *file, int line, int preempt_offset)
> EXPORT_SYMBOL(___might_sleep);
> #endif
>
> +#ifdef CONFIG_DEBUG_UACCESS_SLEEP
> +void __might_resched(const char *file, int line)
> +{
> + if (!unsafe_user_region_active())
> + return;
> +
> + printk(KERN_ERR
> + "BUG: rescheduling function called from user access context at %s:%d\n",
> + file, line);
> + dump_stack();

Since I've been staring intensely at ___might_sleep() lately, I was thinking
we could "copy" it a bit more closely (sorry for going back on what I said
earlier).

Coming back to the double warnings (__might_resched() + schedule_debug()),
it might be better to drop the schedule_debug() warning and just have the
one in __might_resched() - if something goes wrong, there'll already be a
"BUG" in the log.

> +}
> +EXPORT_SYMBOL(__might_resched);
> +#endif
> +
> #ifdef CONFIG_MAGIC_SYSRQ
> void normalize_rt_tasks(void)
> {
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index d4df5b2..d030e31 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2069,6 +2069,14 @@ config IO_STRICT_DEVMEM
>
> If in doubt, say Y.
>
> +config DEBUG_UACCESS_SLEEP
> + bool "Check sleep inside a user access region"
> + depends on DEBUG_KERNEL
> + help
> + If you say Y here, various routines which may sleep will become very
> + noisy if they are called inside a user access region (i.e. between
> + a user_access_begin() and a user_access_end())

If it does get noisy, we should go for some ratelimiting - it's probably
good practice even if it is not noisy actually.

___might_sleep() has this:

if (time_before(jiffies, prev_jiffy + HZ) && prev_jiffy)
return;
prev_jiffy = jiffies;

> +
> source "arch/$(SRCARCH)/Kconfig.debug"
>
> endmenu # Kernel hacking
> --
> 1.9.1
>