Re: Shouldn't set[re]gid look at the group list?

Matthias Urlichs (smurf@noris.de)
30 Oct 1998 11:12:31 +0100


I wrote..:

> I just noticed that setregid() fails for groups which you are a member of,
> but which are not the current group. That is, if id(1) reports
> uid=202(urlichs) gid=90(one) groups=90(one),5(two)
> then setregid(5,5) fails with EPERM.
>
Since I do like the ability to change my default group (permissions-wise it
doesn't make any difference), here's the patch:

Index: exp.15/kernel/sys.c
--- exp.15/kernel/sys.c Tue, 27 Oct 1998 08:59:36 +0100 smurf (kernel_linux/i/16_sys.c 1.56.1.35 644)
+++ exp.15(w)/kernel/sys.c Fri, 30 Oct 1998 11:06:24 +0100 smurf (kernel_linux/i/16_sys.c 1.56.1.35 644)
@@ -264,6 +264,44 @@
* SMP: There are not races, the GIDs are checked only by filesystem
* operations (as far as semantic preservation is concerned).
*/
+
+static int in_groups(gid_t grp)
+{
+ int i = current->ngroups;
+ if (i) {
+ gid_t *groups = current->groups;
+ do {
+ if (*groups == grp)
+ goto out;
+ groups++;
+ i--;
+ } while (i);
+ }
+ return 0;
+out:
+ return 1;
+}
+
+int in_group_p(gid_t grp)
+{
+ if (grp != current->fsgid) {
+ int i = current->ngroups;
+ if (i) {
+ gid_t *groups = current->groups;
+ do {
+ if (*groups == grp)
+ goto out;
+ groups++;
+ i--;
+ } while (i);
+ }
+ return 0;
+ }
+out:
+ return 1;
+}
+
+
asmlinkage int sys_setregid(gid_t rgid, gid_t egid)
{
int old_rgid = current->gid;
@@ -272,6 +310,7 @@
if (rgid != (gid_t) -1) {
if ((old_rgid == rgid) ||
(current->egid==rgid) ||
+ in_groups(rgid) ||
capable(CAP_SETGID))
current->gid = rgid;
else
@@ -281,6 +320,7 @@
if ((old_rgid == egid) ||
(current->egid == egid) ||
(current->sgid == egid) ||
+ in_groups(egid) ||
capable(CAP_SETGID))
current->fsgid = current->egid = egid;
else {
@@ -292,7 +332,7 @@
(egid != (gid_t) -1 && egid != old_rgid))
current->sgid = current->egid;
current->fsgid = current->egid;
- if (current->egid != old_egid)
+ if (current->egid != old_egid && !in_groups(current->egid))
current->dumpable = 0;
return 0;
}
@@ -308,7 +348,7 @@

if (capable(CAP_SETGID))
current->gid = current->egid = current->sgid = current->fsgid = gid;
- else if ((gid == current->gid) || (gid == current->sgid))
+ else if ((gid == current->gid) || (gid == current->sgid) || in_groups(gid))
current->egid = current->fsgid = gid;
else
return -EPERM;
@@ -799,25 +839,6 @@
return -EFAULT;
current->ngroups = gidsetsize;
return 0;
-}
-
-int in_group_p(gid_t grp)
-{
- if (grp != current->fsgid) {
- int i = current->ngroups;
- if (i) {
- gid_t *groups = current->groups;
- do {
- if (*groups == grp)
- goto out;
- groups++;
- i--;
- } while (i);
- }
- return 0;
- }
-out:
- return 1;
}

/*

-- 
Matthias Urlichs  |  noris network GmbH   |   smurf@noris.de  |  ICQ: 20193661
The quote was selected randomly. Really.    |      http://www.noris.de/~smurf/
-- 
If we can ever make red tape nutritional, we can feed the world.
               --R. Schaeberle [Management Accounting]

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/