Debian Bug report logs - #652443
ioprio_get(2): document who==0

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Kalle Olavi Niemitalo <kon@iki.fi>

To: Debian Bug Tracking System <submit@bugs.debian.org>

Subject: ioprio_get(2): document who==0

Date: Sat, 17 Dec 2011 12:26:37 +0200

[Message part 1 (text/plain, inline)]

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.  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.

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.)

-- System Information:
Debian Release: wheezy/sid
  APT prefers testing
  APT policy: (500, 'testing')
Architecture: amd64 (x86_64)

Kernel: Linux 3.1.0-1-amd64 (SMP w/4 CPU cores)
Locale: LANG=fi_FI.utf8, LC_CTYPE=fi_FI.utf8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages manpages-dev depends on:
ii  manpages  3.32-0.2

manpages-dev recommends no packages.

Versions of packages manpages-dev suggests:
ii  man-db [man-browser]  2.6.0.2-3

-- no debconf information

[Message part 2 (application/pgp-signature, inline)]

Message #10 received at 652443@bugs.debian.org (full text, mbox, reply):

From: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>

To: Kalle Olavi Niemitalo <kon@iki.fi>, 652443@bugs.debian.org

Cc: Jens Axboe <axboe@kernel.dk>, control@bugs.debian.org, Марк Коренберг <socketpair@gmail.com>, Colin McCabe <cmccabe@alumni.cmu.edu>, Michael Kerrisk <mtk.linux.lists@gmail.com>

Subject: Re: Bug#652443: ioprio_get(2): document who==0

Date: Sun, 29 Jul 2012 08:32:49 +0200

[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 <kon@iki.fi> 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/

Message #15 received at 652443@bugs.debian.org (full text, mbox, reply):

From: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>

To: Jens Axboe <axboe@kernel.dk>

Cc: Марк Коренберг <socketpair@gmail.com>, Colin McCabe <cmccabe@alumni.cmu.edu>, Kalle Olavi Niemitalo <kon@iki.fi>, 652443@bugs.debian.org

Subject: Re: Bug#652443: ioprio_get(2): document who==0

Date: Mon, 13 Aug 2012 10:41:14 +0200

Hi Jens,

Would you be able to comment please...

Thanks,

Michael

On Sun, Jul 29, 2012 at 8:32 AM, Michael Kerrisk (man-pages)
<mtk.manpages@gmail.com> 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 <kon@iki.fi> 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/

Message #22 received at 652443-done@bugs.debian.org (full text, mbox, reply):

From: Simon Paillard <spaillard@debian.org>

To: 652443-done@bugs.debian.org

Cc: 691195@bugs.debian.org

Subject: Re: Bug#652443: ioprio_get(2): document who==0

Date: Mon, 22 Oct 2012 22:05:11 +0200

Version: 3.42-1 

On Sun, Jul 29, 2012 at 08:32:49AM +0200, Michael Kerrisk (man-pages) wrote:
> On Sat, Dec 17, 2011 at 11:26 PM, Kalle Olavi Niemitalo <kon@iki.fi> wrote:
> > 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.
> >
> > 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.

Documented by Michael in 82fdd7c7d0, already in manpages 3.42.

> >> IOPRIO_WHO_USER
> >>        who is a user ID identifying all of the processes that
> >>        have a matching real UID.
> >
> > 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.)

Remaning items as http://bugs.debian.org/691195 

-- 
Simon Paillard

Send a report that this bug log contains spam.