Messages in this thread Patch in this message | | | Date | Thu, 27 May 2010 19:15:30 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 1/3] sys_personality: validate personality before set_personality() |
| |
On 05/27, Linus Torvalds wrote: > > On Thu, 27 May 2010, Oleg Nesterov wrote: > > > > --- 34-rc1/kernel/exec_domain.c~1_CK_OVERFLOW_EARLIER 2009-04-06 00:03:42.000000000 +0200 > > +++ 34-rc1/kernel/exec_domain.c 2010-05-27 15:15:12.000000000 +0200 > > @@ -193,9 +193,9 @@ SYSCALL_DEFINE1(personality, u_long, per > > u_long old = current->personality; > > > > if (personality != 0xffffffff) { > > - set_personality(personality); > > - if (current->personality != personality) > > + if ((unsigned int)personality != personality) > > return -EINVAL; > > + set_personality(personality); > > } > > I think this is total random noise. The whole type system is crazy - don't > just paper over it.
Of course! I agree very much.
> And if we decide that the field must fit in an unsigned int (reasonable), > then let's just ignore the top bits, and make it work right even if > somebody passes in an unsigned int!
Certainly, this was my first thought.
But I didn't dare to do this change because it is obviously user-visible, and while this is not very important, we should change the declaration of personality() in /usr/include/sys/personality.h
> -SYSCALL_DEFINE1(personality, u_long, personality) > +SYSCALL_DEFINE1(personality, unsigned int, personality)
Indeed!
But. Suppose an application does personality(0xffffffff << 32) on x86_64.
Before this patch we return -EINVAL (but wrongly change ->personality). After this patch this is equal to personality(0), right?
If you think this is fine - I agree. In case we have a bug report we know who should be blamed ;)
As for 2/3 - once again, I think this is user-space problem, but I can't explain this to the bug-reportes.
> - u_long old = current->personality; > + unsigned int old = current->personality; > > if (personality != 0xffffffff) { > set_personality(personality); > @@ -198,7 +198,7 @@ SYSCALL_DEFINE1(personality, u_long, personality) > return -EINVAL;
You can also remove this "return -EINVAL", this is no longer possible.
Oleg.
| |