lkml.org 
[lkml]   [2010]   [May]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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.



\
 
 \ /
  Last update: 2010-05-27 19:19    [W:0.081 / U:0.832 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site