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: Mon, 4 Feb 2019 13:27:17 +0000 From: Julien Thierry <julien.thierry@....com> To: Valentin Schneider <valentin.schneider@....com>, linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org Cc: mingo@...hat.com, peterz@...radead.org, catalin.marinas@....com, will.deacon@....com, james.morse@....com, hpa@...or.com Subject: Re: [PATCH v3 3/4] uaccess: Check no rescheduling function is called in unsafe region On 30/01/2019 16:58, Valentin Schneider wrote: > 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. > My only worry with that approach is that if someone has a function that does resched but does not include the annotation __might_resched() we'd miss the fact that something went wrong. But that might be a lot of "if"s since that assumes that something does go wrong. Could I have a maintainers opinion on this to know if I respin it? >> +} >> +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; > I guess the noisiness could depend on the arch specific handling of user accesses. So yes I guess it might be a good idea to add this. Thanks, -- Julien Thierry
Powered by blists - more mailing lists