* Re: sched: Disallow sched_attr::sched_policy < 0 [not found] <20140602021319.9A5F9660C19@gitolite.kernel.org> @ 2014-06-02 20:22 ` Dave Jones 2014-06-03 8:08 ` Peter Zijlstra 0 siblings, 1 reply; 7+ messages in thread From: Dave Jones @ 2014-06-02 20:22 UTC (permalink / raw) To: Linux Kernel Mailing List; +Cc: mtk.manpages, peterz, torvalds, mingo On Mon, Jun 02, 2014 at 02:13:19AM +0000, Linux Kernel wrote: > sched: Disallow sched_attr::sched_policy < 0 > > The scheduler uses policy=-1 to preserve the current policy state to > implement sys_sched_setparam(), this got exposed to userspace by > accident through sys_sched_setattr(), cure this. > > --- > kernel/sched/core.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index f2205f0..cdefcf7 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -3662,6 +3662,9 @@ SYSCALL_DEFINE3(sched_setattr, pid_t, pid, struct sched_attr __user *, uattr, > if (retval) > return retval; > > + if (attr.sched_policy < 0) > + return -EINVAL; > + > rcu_read_lock(); > retval = -ESRCH; > p = find_process_by_pid(pid); Todays coverity run picked up.. 3687 >>> CID 1219934: Unsigned compared against 0 (NO_EFFECT) >>> This less-than-zero comparison of an unsigned value is never true. "attr.sched_policy < 0U". 3688 if (attr.sched_policy < 0) 3689 return -EINVAL; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: sched: Disallow sched_attr::sched_policy < 0 2014-06-02 20:22 ` sched: Disallow sched_attr::sched_policy < 0 Dave Jones @ 2014-06-03 8:08 ` Peter Zijlstra 2014-06-03 8:15 ` Richard Weinberger 2014-06-03 16:30 ` Linus Torvalds 0 siblings, 2 replies; 7+ messages in thread From: Peter Zijlstra @ 2014-06-03 8:08 UTC (permalink / raw) To: Dave Jones, Linux Kernel Mailing List, mtk.manpages, torvalds, mingo [-- Attachment #1: Type: text/plain, Size: 1431 bytes --] On Mon, Jun 02, 2014 at 04:22:04PM -0400, Dave Jones wrote: > On Mon, Jun 02, 2014 at 02:13:19AM +0000, Linux Kernel wrote: > > > sched: Disallow sched_attr::sched_policy < 0 > > > > The scheduler uses policy=-1 to preserve the current policy state to > > implement sys_sched_setparam(), this got exposed to userspace by > > accident through sys_sched_setattr(), cure this. > > > > --- > > kernel/sched/core.c | 3 +++ > > 1 files changed, 3 insertions(+), 0 deletions(-) > > > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > > index f2205f0..cdefcf7 100644 > > --- a/kernel/sched/core.c > > +++ b/kernel/sched/core.c > > @@ -3662,6 +3662,9 @@ SYSCALL_DEFINE3(sched_setattr, pid_t, pid, struct sched_attr __user *, uattr, > > if (retval) > > return retval; > > > > + if (attr.sched_policy < 0) > > + return -EINVAL; > > + > > rcu_read_lock(); > > retval = -ESRCH; > > p = find_process_by_pid(pid); > > Todays coverity run picked up.. > > 3687 > >>> CID 1219934: Unsigned compared against 0 (NO_EFFECT) > >>> This less-than-zero comparison of an unsigned value is never true. "attr.sched_policy < 0U". > 3688 if (attr.sched_policy < 0) > 3689 return -EINVAL; > Once upon a time GCC also did warns like that, but my compiler is silent :-( Yes, that needs fixing.. [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: sched: Disallow sched_attr::sched_policy < 0 2014-06-03 8:08 ` Peter Zijlstra @ 2014-06-03 8:15 ` Richard Weinberger 2014-06-03 8:32 ` Peter Zijlstra 2014-06-03 16:30 ` Linus Torvalds 1 sibling, 1 reply; 7+ messages in thread From: Richard Weinberger @ 2014-06-03 8:15 UTC (permalink / raw) To: Peter Zijlstra Cc: Dave Jones, Linux Kernel Mailing List, Michael Kerrisk-manpages, Linus Torvalds, Ingo Molnar On Tue, Jun 3, 2014 at 10:08 AM, Peter Zijlstra <peterz@infradead.org> wrote: > On Mon, Jun 02, 2014 at 04:22:04PM -0400, Dave Jones wrote: >> On Mon, Jun 02, 2014 at 02:13:19AM +0000, Linux Kernel wrote: >> >> > sched: Disallow sched_attr::sched_policy < 0 >> > >> > The scheduler uses policy=-1 to preserve the current policy state to >> > implement sys_sched_setparam(), this got exposed to userspace by >> > accident through sys_sched_setattr(), cure this. >> > >> > --- >> > kernel/sched/core.c | 3 +++ >> > 1 files changed, 3 insertions(+), 0 deletions(-) >> > >> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c >> > index f2205f0..cdefcf7 100644 >> > --- a/kernel/sched/core.c >> > +++ b/kernel/sched/core.c >> > @@ -3662,6 +3662,9 @@ SYSCALL_DEFINE3(sched_setattr, pid_t, pid, struct sched_attr __user *, uattr, >> > if (retval) >> > return retval; >> > >> > + if (attr.sched_policy < 0) >> > + return -EINVAL; >> > + >> > rcu_read_lock(); >> > retval = -ESRCH; >> > p = find_process_by_pid(pid); >> >> Todays coverity run picked up.. >> >> 3687 >> >>> CID 1219934: Unsigned compared against 0 (NO_EFFECT) >> >>> This less-than-zero comparison of an unsigned value is never true. "attr.sched_policy < 0U". >> 3688 if (attr.sched_policy < 0) >> 3689 return -EINVAL; >> > > Once upon a time GCC also did warns like that, but my compiler is silent > :-( > > Yes, that needs fixing.. I sent already a patch for that... -- Thanks, //richard ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: sched: Disallow sched_attr::sched_policy < 0 2014-06-03 8:15 ` Richard Weinberger @ 2014-06-03 8:32 ` Peter Zijlstra 2014-06-03 8:47 ` Richard Weinberger 0 siblings, 1 reply; 7+ messages in thread From: Peter Zijlstra @ 2014-06-03 8:32 UTC (permalink / raw) To: Richard Weinberger Cc: Dave Jones, Linux Kernel Mailing List, Michael Kerrisk-manpages, Linus Torvalds, Ingo Molnar [-- Attachment #1: Type: text/plain, Size: 266 bytes --] On Tue, Jun 03, 2014 at 10:15:44AM +0200, Richard Weinberger wrote: > > Once upon a time GCC also did warns like that, but my compiler is silent > > :-( > > > > Yes, that needs fixing.. > > I sent already a patch for that... I found it.. thanks again :-) [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: sched: Disallow sched_attr::sched_policy < 0 2014-06-03 8:32 ` Peter Zijlstra @ 2014-06-03 8:47 ` Richard Weinberger 2014-06-03 13:16 ` Dave Jones 0 siblings, 1 reply; 7+ messages in thread From: Richard Weinberger @ 2014-06-03 8:47 UTC (permalink / raw) To: Peter Zijlstra Cc: Dave Jones, Linux Kernel Mailing List, Michael Kerrisk-manpages, Linus Torvalds, Ingo Molnar [-- Attachment #1: Type: text/plain, Size: 482 bytes --] Am 03.06.2014 10:32, schrieb Peter Zijlstra: > On Tue, Jun 03, 2014 at 10:15:44AM +0200, Richard Weinberger wrote: >>> Once upon a time GCC also did warns like that, but my compiler is silent >>> :-( -Wtype-limits is what you're looking for. /me currently builds some kernel configs to find out how much noise it triggers... >>> Yes, that needs fixing.. >> >> I sent already a patch for that... > > I found it.. thanks again :-) > :-) Thanks, //richard [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 901 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: sched: Disallow sched_attr::sched_policy < 0 2014-06-03 8:47 ` Richard Weinberger @ 2014-06-03 13:16 ` Dave Jones 0 siblings, 0 replies; 7+ messages in thread From: Dave Jones @ 2014-06-03 13:16 UTC (permalink / raw) To: Richard Weinberger Cc: Peter Zijlstra, Linux Kernel Mailing List, Michael Kerrisk-manpages, Linus Torvalds, Ingo Molnar On Tue, Jun 03, 2014 at 10:47:18AM +0200, Richard Weinberger wrote: > Am 03.06.2014 10:32, schrieb Peter Zijlstra: > > On Tue, Jun 03, 2014 at 10:15:44AM +0200, Richard Weinberger wrote: > >>> Once upon a time GCC also did warns like that, but my compiler is silent > >>> :-( > > -Wtype-limits is what you're looking for. > > /me currently builds some kernel configs to find out how much noise it triggers... Probably quite a bit I'd bet. There's a load of similar bugs in coverity's db. Some of them look benign, and are probably there just in case someone ever changes the type of a var, but it's non-obvious sometimes if the values a function receives can ever actually be < 0 Dave ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: sched: Disallow sched_attr::sched_policy < 0 2014-06-03 8:08 ` Peter Zijlstra 2014-06-03 8:15 ` Richard Weinberger @ 2014-06-03 16:30 ` Linus Torvalds 1 sibling, 0 replies; 7+ messages in thread From: Linus Torvalds @ 2014-06-03 16:30 UTC (permalink / raw) To: Peter Zijlstra Cc: Dave Jones, Linux Kernel Mailing List, Michael Kerrisk-manpages, Ingo Molnar On Tue, Jun 3, 2014 at 1:08 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > Once upon a time GCC also did warns like that, but my compiler is silent > :-( You should be happy. The gcc warnings were shit. Iirc, gcc literally at one point warned about things like unsigned int i; if (i < 5) because that's comparing an unsigned type ("i") with an expression having a signed type ("5"). Yes, technically true, but it's not actually a useful warning. That got fixed pretty quickly, but I think gcc *still* warns about things like unsigned int i; if (i >= 0 && i <= 6) ... which is actually a very valid thing to do, and is commonly the result of using a range-checking macro, or in general writing code so that it is robust and doesn't care about the actual underlying type. Warnings about robust code are f*cking broken, and easily worse than not having the warning at all. Because it results in people removing the range check. Btw, -Wsign-compare still complains about int i; if (i < 0 || i > sizeof(i)) return error; which is another example of a f*cking broken warning. There is no way to avoid that warning without making the code worse. That code is _correct_, dammit, and anybody who thinks it should warn (or the programmer should cast the sizeof to "int") is a tool and a moron. End result: disabling "-Wsign-compare" is thus the only correct thing to do. Sadly compiler writers don't seem to care too deeply about the sanity of their warnings. Linus ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-06-03 16:30 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20140602021319.9A5F9660C19@gitolite.kernel.org>
2014-06-02 20:22 ` sched: Disallow sched_attr::sched_policy < 0 Dave Jones
2014-06-03 8:08 ` Peter Zijlstra
2014-06-03 8:15 ` Richard Weinberger
2014-06-03 8:32 ` Peter Zijlstra
2014-06-03 8:47 ` Richard Weinberger
2014-06-03 13:16 ` Dave Jones
2014-06-03 16:30 ` Linus Torvalds
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox