Prev: Staging: dream: pmem.c: Fix checkpatch.pl issues.
Next: fs/minix: bugfix, number of indirect block ptrs per block depends on block size
From: Oleg Nesterov on 27 May 2010 13:20
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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andi Kleen on 27 May 2010 14:20
On Thu, May 27, 2010 at 10:51:46AM -0700, Linus Torvalds wrote:
>
>
> On Thu, 27 May 2010, Oleg Nesterov wrote:
> >
> > 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?
>
> Yes. And I'm willing to take that "risk" in the name of not having to
> carry crazy stuff around in the kernel.

Perhaps we can have a personality with the old personality behaviour @)

Just joking, I doubt anything really cares. I don't think personality
was used much in 64bit except for uname emulation.

-Andi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Oleg Nesterov on 27 May 2010 14:20
On 05/27, Linus Torvalds wrote:
>
>
> On Thu, 27 May 2010, Oleg Nesterov wrote:
> >
> > You can also remove this "return -EINVAL", this is no longer possible.
>
> I've already removed the patch from my tree, I wasn't going to commit it
> without somebody testing it. So maybe you could re-do the series with that
> cleanup too?

Sure, will do tomorrow.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: Staging: dream: pmem.c: Fix checkpatch.pl issues.
Next: fs/minix: bugfix, number of indirect block ptrs per block depends on block size