Hi Jens, Would you be able to comment please...
Thanks, Michael On Sun, Jul 29, 2012 at 8:32 AM, Michael Kerrisk (man-pages) <[email protected]> wrote: > [CCing Jens because of the discussion below about IOPRIO_WHO_USER > below; he may have a comment] > [CCing Марк, who independently noted the lack of documentation for > IOPRIO_WHO_PROCESS, who==0 .] > [CCing Colin McCabe who sent other recent fixes for the ioprio_set.2 page] > > On Sat, Dec 17, 2011 at 11:26 PM, Kalle Olavi Niemitalo <[email protected]> wrote: >> Package: manpages-dev >> Version: 3.32-0.2 >> Severity: wishlist >> File: /usr/share/man/man2/ioprio_get.2.gz >> >> The ioprio_get(2) manual page describes the meanings of the which >> and who parameters: >> >>> IOPRIO_WHO_PROCESS >>> who is a process ID identifying a single process. >>> >>> IOPRIO_WHO_PGRP >>> who is a process group ID identifying all the members of >>> a process group. >>> >>> IOPRIO_WHO_USER >>> who is a user ID identifying all of the processes that >>> have a matching real UID. >> >> The manual page should mention that IOPRIO_WHO_PROCESS and >> IOPRIO_WHO_PGRP also allow who==0. > > Yes. > >> As implemented in >> fs/ioprio.c, who==0 means the calling process or its process >> group. The ioprio program in util-linux already uses the >> feature. This is worth documenting separately because >> e.g. tcsetpgrp does not treat pgrp==0 in that way. > > Agreed, this should be documented since various APIs interpret pgrp==0 > differently. Some (e.g., killpg(2)) are like this syscall, others are > not. > >> For IOPRIO_WHO_USER, the situation is more complex: who==0 means >> the root user in ioprio_set but the current user (I think the >> real UID of the calling process) in ioprio_get. (That >> inconsistency might even be a bug.) > > So, I'm not sure I'm following the kernel code too well here... > @Jens, your comments would be very welovem. > > In ioprio_get() (Linux 3.5 kernel source file fs/ioprio.c), I see the > following: > > case IOPRIO_WHO_USER: > uid = make_kuid(current_user_ns(), who); > if (!who) > user = current_user(); > else > user = find_user(uid); > > if (!user) > break; > > do_each_thread(g, p) { > if (!uid_eq(task_uid(p), user->uid)) > continue; > tmpio = get_task_ioprio(p); > if (tmpio < 0) > continue; > if (ret == -ESRCH) > ret = tmpio; > else > ret = ioprio_best(ret, tmpio); > } while_each_thread(g, p); > > if (who) > free_uid(user); > break; > > In ioprio_set(), I see: > > case IOPRIO_WHO_USER: > uid = make_kuid(current_user_ns(), who); > if (!uid_valid(uid)) > break; > if (!who) > user = current_user(); > else > user = find_user(uid); > > if (!user) > break; > > do_each_thread(g, p) { > if (!uid_eq(task_uid(p), uid)) > continue; > ret = set_task_ioprio(p, ioprio); > if (ret) > goto free_uid; > } while_each_thread(g, p); > free_uid: > if (who) > free_uid(user); > break; > > This suggests to me that you are right Kalle, in your interpretation > of who==0 for the IOPRIO_WHO_USER, since ioprio_get() uses > current_iser()->uid for its scan while ioprio_get() uses the UID > returned by make_kuid() (So, to be precise, I think who==0 in this > case means "the UID of the uer who is thye super user in this user > namespace"). If that's correct, it does of course need to be > documented. I'd be happy to get confirmation from Jens on this point. > > I suppose that the differing meaning of who==0 for IOPRIO_WHO_USER in > ioprio_get() versus ioprio_set() is by design. But if so, like you > Kalle, I agree that it's a design point that is likely to surprise > users (and surprises here might have security implications). Again, > I'd like to get input from Jens. > > In the meantime, I've applied the patch below to cover the other two cases. > > Thanks, > > Michael > > --- a/man2/ioprio_set.2 > +++ b/man2/ioprio_set.2 > @@ -56,10 +56,16 @@ is interpreted, and has one of the following values: > .B IOPRIO_WHO_PROCESS > .I who > is a process ID or thread ID identifying a single process or thread. > +If > +.I who > +is 0, then operate on the calling thread. > .TP > .B IOPRIO_WHO_PGRP > .I who > is a process group ID identifying all the members of a process group. > +If > +.I who > +is 0, then operate on the process group of which the caller is a member. > .TP > .B IOPRIO_WHO_USER > .I who > > -- > Michael Kerrisk > Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ > Author of "The Linux Programming Interface"; http://man7.org/tlpi/ -- Michael Kerrisk Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ Author of "The Linux Programming Interface"; http://man7.org/tlpi/ -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected]
