Re: [patch] fix NPTL thread iterator construct in cap_set_pg()

["Andrew G. Morgan" <morgan@kernel.org>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ken,

In 2.2.0, pre-LSM and NPTL, this code looked like this:

     read_lock(&tasklist_lock);
     for_each_task(target) {
             if (target->pgrp != pgrp)
                     continue;
             target->cap_effective   = *effective;
             target->cap_inheritable = *inheritable;
             target->cap_permitted   = *permitted;
     }
     read_unlock(&tasklist_lock);

that is, if a process is in the process group, it got its capabilities
changed.

So far as I can tell, I've not touched this code since it looked like
the above, so I can't really comment on the motivation for the thread
iterator changes. I'm also going to have to read up on NPTL to say
anything credible on this front.

In general, its very hard to get a system state in which this code is
run, so it wouldn't surprise me if these thread iterator changes have
never been tested.

Finally, I've very much in favor of deleting any code that modifies the
capabilities of another process/thread; see the #if[n]def
CONFIG_SECURITY_FILE_CAPABILITIES sections of this file, so any fix for
this (which, if needed, should be adopted) may also be short-lived...

Cheers

Andrew

Ken Chen wrote:
> The usage of while_each_pid_task() construct in cap_set_pg() looks incorrect.
> The macro is meant to form 'do ... while' loop instead of a simple while loop.
> I think currently it will skip thread leader of a NPTL process.
> 
> Fix by convert to 'do ... while' style.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFIqLRO+bHCR3gb8jsRAnFGAJoDm9VxcpmOYqEuaWmaVrnNWkb9owCgzOPH
6tG3n4GMsW8oNX4MiNiJ3FQ=
=obJY
-----END PGP SIGNATURE-----