Linus,
Please pull the latest sched-fixes-for-linus git tree from:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git sched-fixes-for-linus
Thanks,
Ingo
------------------>
Anton Blanchard (1):
sched: Fix sched_getaffinity()
kernel/sched.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/sched.c b/kernel/sched.c
index 528a105..eaf5c73 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -4902,7 +4902,7 @@ SYSCALL_DEFINE3(sched_getaffinity, pid_t, pid, unsigned int, len,
int ret;
cpumask_var_t mask;
- if (len < nr_cpu_ids)
+ if ((len * BITS_PER_BYTE) < nr_cpu_ids)
return -EINVAL;
if (len & (sizeof(unsigned long)-1))
return -EINVAL;
On Thu, 8 Apr 2010, Ingo Molnar wrote:
>
> - if (len < nr_cpu_ids)
> + if ((len * BITS_PER_BYTE) < nr_cpu_ids)
> return -EINVAL;
Not that it really matters, but this will now fail for no good reason if
you pass it a half-gigabyte area due to overflow.
Of course, if you pass it a half gig memory array, you're a f*cking moron
to begin with, so I don't think anybody really _cares_. But in general,
when checking system call arguments, I'd like people to think about
overflow issues more.
In this case it doesn't matter, and overflow just makes the test more
conservative than they need to be, but when it _does_ matter it often ends
up being a security issue.
Linus
Linus Torvalds <[email protected]> writes:
> On Thu, 8 Apr 2010, Ingo Molnar wrote:
>>
>> - if (len < nr_cpu_ids)
>> + if ((len * BITS_PER_BYTE) < nr_cpu_ids)
>> return -EINVAL;
>
> Not that it really matters, but this will now fail for no good reason if
> you pass it a half-gigabyte area due to overflow.
Which can easily be avoided.
if (len < DIV_ROUND_UP(nr_cpu_ids, BITS_PER_BYTE))
Andreas.
--
Andreas Schwab, [email protected]
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84 5EC7 45C6 250E 6F00 984E
"And now for something completely different."
* Andreas Schwab <[email protected]> wrote:
> Linus Torvalds <[email protected]> writes:
>
> > On Thu, 8 Apr 2010, Ingo Molnar wrote:
> >>
> >> - if (len < nr_cpu_ids)
> >> + if ((len * BITS_PER_BYTE) < nr_cpu_ids)
> >> return -EINVAL;
> >
> > Not that it really matters, but this will now fail for no good reason if
> > you pass it a half-gigabyte area due to overflow.
>
> Which can easily be avoided.
>
> if (len < DIV_ROUND_UP(nr_cpu_ids, BITS_PER_BYTE))
nr_cpu_ids is a signed integer which turns the DIV_ROUND_UP into a somewhat
suboptimal instruction sequence. (havent checked it though)
So i'd suggest changing nr_cpu_ids to unsigned int [unless there's some strong
reason to have it signed] plus doing something like:
if (len < (nr_cpu_ids >> BITS_PER_BYTE_BITS))
ought to both result in better code and should be more readable. We'd have to
add:
#define BITS_PER_BYTE_BITS 3
to linux/bitops.h.
Ingo
On Thu, 8 Apr 2010, Ingo Molnar wrote:
>
> So i'd suggest changing nr_cpu_ids to unsigned int [unless there's some strong
> reason to have it signed] plus doing something like:
>
> if (len < (nr_cpu_ids >> BITS_PER_BYTE_BITS))
No workee.
It really should round up.
If you worry about code generation, I'd suggest looking at whether
nr_cpu_ids could just be made unsigned.
Anyway, this all was _not_ the point of my original email. I really don't
care about this particular instance. I care more about the whole "in
general people should think _way_ more about validating user-supplied
arguments than clearly happened this time".
Linus
* Linus Torvalds <[email protected]> wrote:
>
>
> On Thu, 8 Apr 2010, Ingo Molnar wrote:
> >
> > So i'd suggest changing nr_cpu_ids to unsigned int [unless there's some strong
> > reason to have it signed] plus doing something like:
> >
> > if (len < (nr_cpu_ids >> BITS_PER_BYTE_BITS))
>
> No workee.
>
> It really should round up.
Indeed.
> If you worry about code generation, I'd suggest looking at whether
> nr_cpu_ids could just be made unsigned.
>
> Anyway, this all was _not_ the point of my original email. I really don't
> care about this particular instance. I care more about the whole "in general
> people should think _way_ more about validating user-supplied arguments than
> clearly happened this time".
Yeah, no argument about that, point taken and accepted.
Ingo