[patch] sys_setpriority() euid semantics fix

[Ingo Molnar]

* Peter Williams <pwil3058@xxxxxxxxxxxxxx> wrote:

> I've just noticed what might be a bug in the original code.  Shouldn't
> the following:
> 
>        if ((current->euid != p->euid) && (current->euid != p->uid) &&
>                !capable(CAP_SYS_NICE))
> 
> be:
> 
>        if ((current->uid != p->uid) && (current->euid != p->uid) &&
>                !capable(CAP_SYS_NICE))
> 
> I.e. if the real or effective uid of the task doing the setting is not
> the same as the uid of the target task it is not allowed to change the
> target task's policy unless it is specially privileged.

no. The original code is quite logical: when doing something to others,
only the euid is taken into account. When others do something to you,
both the uid and the euid is checked ('others' might have no idea about
this task temporarily changing the euid to a less/more privileged uid). 
So sys_setscheduler() [and sys_setaffinity(), which does the same] is
fine.

what _is_ inconsistent is kernel/sys.c's setpriority()/set_one_prio(). 

It checks current->euid|uid against p->uid, which makes little sense,
but is how we've been doing it ever since. It's a Linux quirk documented
in the manpage. To make things funnier, SuS requires current->euid|uid
match against p->euid.

The patch below fixes it (and brings the logic in line with what
setscheduler()/setaffinity() does), but if we do it then it should be
done only in 2.6.12 or later, after good exposure in -mm.

(Worst-case this could break an application but i highly doubt it: it at
most could deny renicing another task to positive (or in very rare
cases, to negative) nice values, which no application should crash on
something like that, normally.)

	Ingo

--

fix a setpriority() Linux quirk, implement euid semantics correctly.

Signed-off-by: Ingo Molnar <mingo@xxxxxxx>

--- linux/kernel/sys.c.orig
+++ linux/kernel/sys.c
@@ -216,12 +216,13 @@ int unregister_reboot_notifier(struct no
 }
 
 EXPORT_SYMBOL(unregister_reboot_notifier);
+
 static int set_one_prio(struct task_struct *p, int niceval, int error)
 {
 	int no_nice;
 
 	if (p->uid != current->euid &&
-		p->uid != current->uid && !capable(CAP_SYS_NICE)) {
+		p->euid != current->euid && !capable(CAP_SYS_NICE)) {
 		error = -EPERM;
 		goto out;
 	}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/