Capabilities-related change in 2.5.40

DISCUSSION Daniel Jacobowitz 3 participants 10 messages Oct 1, 2002 – Oct 2, 2002 Capabilities

Share & tools

Daniel Jacobowitz

author

2002-10-01 16:43 UTC #0 lore raw

First of all, I think the LSM code is confused in its use of cap_t. I think that cap_capget should be using to_cap_t instead; it's converting _to_ a kernel_cap_t, right?

Second of all, my login shell (as a user) gets a very bizarre response to sys_capget:

capget(0x19980330, 0, {CAP_CHOWN | CAP_DAC_OVERRIDE | CAP_DAC_READ_SEARCH |
CAP_FOWNER | CAP_FSETID | CAP_KILL | CAP_SETGID | CAP_SETUID | CAP_LINUX_IMMUTABLE | CAP_NET_BIND_SERVICE | CAP_NET_BROADCAST | CAP_NET_ADMIN | CAP_NET_RAW | CAP_IPC_LOCK | CAP_IPC_OWNER | CAP_SYS_MODULE

  | CAP_SYS_RAWIO | CAP_SYS_CHROOT | CAP_SYS_PTRACE | CAP_SYS_PACCT |
  CAP_SYS_ADMIN | CAP_SYS_BOOT | CAP_SYS_NICE | CAP_SYS_RESOURCE |
  CAP_SYS_TIME | CAP_SYS_TTY_CONFIG | 0xf8000000,
  CAP_CHOWN | CAP_DAC_OVERRIDE
  | CAP_DAC_READ_SEARCH | CAP_FOWNER | CAP_FSETID | CAP_KILL | CAP_SETGID |
  CAP_SETUID | CAP_SETPCAP | CAP_LINUX_IMMUTABLE | CAP_NET_BIND_SERVICE |
  CAP_NET_BROADCAST | CAP_NET_ADMIN | CAP_NET_RAW | CAP_IPC_LOCK |
  CAP_IPC_OWNER | CAP_SYS_MODULE | CAP_SYS_RAWIO | CAP_SYS_CHROOT |
  CAP_SYS_PTRACE | CAP_SYS_PACCT | CAP_SYS_ADMIN | CAP_SYS_BOOT | CAP_SYS_NICE
  | CAP_SYS_RESOURCE | CAP_SYS_TIME | CAP_SYS_TTY_CONFIG | 0xf8000000,}) = 0

The reason? cap_get_proc has always been broken. But the capability set of task 0, swapper, has now changed. It used to be empty. So, I'll go report this to libcap. The change in capabilities for swapper is presumably benign.

Signature · 2 lines

Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Chris Wright

2002-10-01 20:47 UTC #1 lore raw

Quoted 3 lines · * Daniel Jacobowitz ([email protected]) wrote:

First of all, I think the LSM code is confused in its use of cap_t. I think that cap_capget should be using to_cap_t instead; it's converting _to_ a kernel_cap_t, right?

I believe capget should be using cap_t (it's been this way since 2.2). The macros are kind of meaningless unless you have defined STRICT_CAP_T_TYPECHECKS. And anyway, cap_t is used to extract a cap (to a __u32) from a structure. While to_cap_t is used to place a __u32 in a cap structure. capget is retrieving the value from a structure and simply placing it in a __u32 that is copied back to userspace. However, there is a merge error there with duplicate code.

capability.c renamed from kernel/capability.c +0 -3

@@ -60,9 +60,6 @@
           goto out;
      }
 
-     data.permitted = cap_t(target->cap_permitted);
-     data.inheritable = cap_t(target->cap_inheritable); 
-     data.effective = cap_t(target->cap_effective);
      ret = security_ops->capget(target, &data.effective, &data.inheritable, &data.permitted);
 
 out:

Quoted 17 lines

Second of all, my login shell (as a user) gets a very bizarre response to sys_capget:
capget(0x19980330, 0, {CAP_CHOWN | CAP_DAC_OVERRIDE | CAP_DAC_READ_SEARCH |
CAP_FOWNER | CAP_FSETID | CAP_KILL | CAP_SETGID | CAP_SETUID | CAP_LINUX_IMMUTABLE | CAP_NET_BIND_SERVICE | CAP_NET_BROADCAST | CAP_NET_ADMIN | CAP_NET_RAW | CAP_IPC_LOCK | CAP_IPC_OWNER | CAP_SYS_MODULE
  | CAP_SYS_RAWIO | CAP_SYS_CHROOT | CAP_SYS_PTRACE | CAP_SYS_PACCT |
  CAP_SYS_ADMIN | CAP_SYS_BOOT | CAP_SYS_NICE | CAP_SYS_RESOURCE |
  CAP_SYS_TIME | CAP_SYS_TTY_CONFIG | 0xf8000000,
  CAP_CHOWN | CAP_DAC_OVERRIDE
  | CAP_DAC_READ_SEARCH | CAP_FOWNER | CAP_FSETID | CAP_KILL | CAP_SETGID |
  CAP_SETUID | CAP_SETPCAP | CAP_LINUX_IMMUTABLE | CAP_NET_BIND_SERVICE |
  CAP_NET_BROADCAST | CAP_NET_ADMIN | CAP_NET_RAW | CAP_IPC_LOCK |
  CAP_IPC_OWNER | CAP_SYS_MODULE | CAP_SYS_RAWIO | CAP_SYS_CHROOT |
  CAP_SYS_PTRACE | CAP_SYS_PACCT | CAP_SYS_ADMIN | CAP_SYS_BOOT | CAP_SYS_NICE
  | CAP_SYS_RESOURCE | CAP_SYS_TIME | CAP_SYS_TTY_CONFIG | 0xf8000000,}) = 0

Quoted 4 lines

The reason? cap_get_proc has always been broken. But the capability set of task 0, swapper, has now changed. It used to be empty. So, I'll go report this to libcap. The change in capabilities for swapper is presumably benign.

I'm not sure what you are pointing out? Is cap_get_proc using header.pid = 0 regardless? Hmm, yes.

cap_get_proc()

	cap_init()
		malloc
		memset(0)
		head.version = _LINUX_CAPABILITY_VERSION
	capget(&head, &set)	<-- head.pid = 0

Also, I've just looked at 2.2, 2.4, 2.5 and they all have the same caps for INIT_TASK:

2.2

#define CAP_FULL_SET        to_cap_t(~0)
#define CAP_INIT_EFF_SET    to_cap_t(~0 & ~CAP_TO_MASK(CAP_SETPCAP))
#define CAP_INIT_INH_SET    to_cap_t(0)

/* caps */ CAP_INIT_EFF_SET,CAP_INIT_INH_SET,CAP_FULL_SET, \
/* keep_caps */ 0, \

2.4

#define CAP_FULL_SET        to_cap_t(~0)
#define CAP_INIT_EFF_SET    to_cap_t(~0 & ~CAP_TO_MASK(CAP_SETPCAP))
#define CAP_INIT_INH_SET    to_cap_t(0)
    cap_effective:	CAP_INIT_EFF_SET,	\
    cap_inheritable:	CAP_INIT_INH_SET,	\
    cap_permitted:	CAP_FULL_SET,		\
    keep_capabilities:	0,			\

2.5

#define CAP_FULL_SET        to_cap_t(~0)
#define CAP_INIT_EFF_SET    to_cap_t(~0 & ~CAP_TO_MASK(CAP_SETPCAP))
#define CAP_INIT_INH_SET    to_cap_t(0)
	.cap_effective  = CAP_INIT_EFF_SET,	\
	.cap_inheritable = CAP_INIT_INH_SET,	\
	.cap_permitted  = CAP_FULL_SET,		\
	.keep_capabilities = 0,			\

thanks,
-chris

Signature · 1 line

Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Daniel Jacobowitz

author

2002-10-01 21:06 UTC #2 lore raw

Quoted 25 lines · Chris Wright

Quoted 17 lines
Second of all, my login shell (as a user) gets a very bizarre response to sys_capget:
capget(0x19980330, 0, {CAP_CHOWN | CAP_DAC_OVERRIDE | CAP_DAC_READ_SEARCH |
CAP_FOWNER | CAP_FSETID | CAP_KILL | CAP_SETGID | CAP_SETUID | CAP_LINUX_IMMUTABLE | CAP_NET_BIND_SERVICE | CAP_NET_BROADCAST | CAP_NET_ADMIN | CAP_NET_RAW | CAP_IPC_LOCK | CAP_IPC_OWNER | CAP_SYS_MODULE
  | CAP_SYS_RAWIO | CAP_SYS_CHROOT | CAP_SYS_PTRACE | CAP_SYS_PACCT |
  CAP_SYS_ADMIN | CAP_SYS_BOOT | CAP_SYS_NICE | CAP_SYS_RESOURCE |
  CAP_SYS_TIME | CAP_SYS_TTY_CONFIG | 0xf8000000,
  CAP_CHOWN | CAP_DAC_OVERRIDE
  | CAP_DAC_READ_SEARCH | CAP_FOWNER | CAP_FSETID | CAP_KILL | CAP_SETGID |
  CAP_SETUID | CAP_SETPCAP | CAP_LINUX_IMMUTABLE | CAP_NET_BIND_SERVICE |
  CAP_NET_BROADCAST | CAP_NET_ADMIN | CAP_NET_RAW | CAP_IPC_LOCK |
  CAP_IPC_OWNER | CAP_SYS_MODULE | CAP_SYS_RAWIO | CAP_SYS_CHROOT |
  CAP_SYS_PTRACE | CAP_SYS_PACCT | CAP_SYS_ADMIN | CAP_SYS_BOOT | CAP_SYS_NICE
  | CAP_SYS_RESOURCE | CAP_SYS_TIME | CAP_SYS_TTY_CONFIG | 0xf8000000,}) = 0
Quoted 4 lines
The reason? cap_get_proc has always been broken. But the capability set of task 0, swapper, has now changed. It used to be empty. So, I'll go report this to libcap. The change in capabilities for swapper is presumably benign.
I'm not sure what you are pointing out? Is cap_get_proc using header.pid = 0 regardless? Hmm, yes.

Quoted 7 lines

cap_get_proc()

	cap_init()
		malloc
		memset(0)
		head.version = _LINUX_CAPABILITY_VERSION
	capget(&head, &set)	<-- head.pid = 0

Yes. It was pointed out to me that libcap2 snapshots behave correctly.

Quoted 19 lines

Also, I've just looked at 2.2, 2.4, 2.5 and they all have the same caps for INIT_TASK:

2.2

#define CAP_FULL_SET        to_cap_t(~0)
#define CAP_INIT_EFF_SET    to_cap_t(~0 & ~CAP_TO_MASK(CAP_SETPCAP))
#define CAP_INIT_INH_SET    to_cap_t(0)

/* caps */ CAP_INIT_EFF_SET,CAP_INIT_INH_SET,CAP_FULL_SET, \
/* keep_caps */ 0, \

2.4

#define CAP_FULL_SET        to_cap_t(~0)
#define CAP_INIT_EFF_SET    to_cap_t(~0 & ~CAP_TO_MASK(CAP_SETPCAP))
#define CAP_INIT_INH_SET    to_cap_t(0)
    cap_effective:	CAP_INIT_EFF_SET,	\
    cap_inheritable:	CAP_INIT_INH_SET,	\
    cap_permitted:	CAP_FULL_SET,		\
    keep_capabilities:	0,			\

Quoted 8 lines

2.5

#define CAP_FULL_SET        to_cap_t(~0)
#define CAP_INIT_EFF_SET    to_cap_t(~0 & ~CAP_TO_MASK(CAP_SETPCAP))
#define CAP_INIT_INH_SET    to_cap_t(0)
	.cap_effective  = CAP_INIT_EFF_SET,	\
	.cap_inheritable = CAP_INIT_INH_SET,	\
	.cap_permitted  = CAP_FULL_SET,		\
	.keep_capabilities = 0,			\

Not init: swapper. Try it on 2.4:
drow@nevyn:~% getpcaps 0
Capabilities for `0': =

2.5.40 gives me a very different answer :)

Signature · 2 lines

Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Chris Wright

2002-10-02 07:40 UTC #3 lore raw

Quoted 2 lines · * Daniel Jacobowitz ([email protected]) wrote:

Yes. It was pointed out to me that libcap2 snapshots behave correctly.

Ah, thanks for the info. Hmm, libcap2 still looks like it sets up the header with pid == 0. Maybe I'm missing something.

Quoted 1 line

Not init: swapper.

Yes, although INIT_TASK sets up the task_struct for swapper.

Quoted 5 lines

Try it on 2.4:
drow@nevyn:~% getpcaps 0
Capabilities for `0': =
2.5.40 gives me a very different answer :)

Heh, you're right. However, 2.5.20 behaves the same as 2.4. Looking back this appears to be caused by 2.5.21 locking cleanups done by rml. The older code interpreted pid == 0 to mean current, whereas the new code unconditionally does find_task_by_pid(0). This patch fixes that, and then pid == 0 from libcap should work again.

kernel/capability.c +9 ~~-10~~

@@ -33,7 +33,7 @@
      int ret = 0;
      pid_t pid;
      __u32 version;
-     task_t *target;
+     task_t *target = current;
      struct __user_cap_data_struct data;
 
      if (get_user(version, &header->version))

@@ -52,21 +52,20 @@
              return -EINVAL;
 
      spin_lock(&task_capability_lock);
-     read_lock(&tasklist_lock); 
 
-     target = find_task_by_pid(pid);
-     if (!target) {
-          ret = -ESRCH;
-          goto out;
+     if (pid && pid != current->pid) {
+     	read_lock(&tasklist_lock); 
+     	target = find_task_by_pid(pid);
+     	read_unlock(&tasklist_lock); 
+     	if (!target) {
+          	ret = -ESRCH;
+          	goto out;
+	}
      }
 
-     data.permitted = cap_t(target->cap_permitted);
-     data.inheritable = cap_t(target->cap_inheritable); 
-     data.effective = cap_t(target->cap_effective);
      ret = security_ops->capget(target, &data.effective, &data.inheritable, &data.permitted);
 
 out:
-     read_unlock(&tasklist_lock); 
      spin_unlock(&task_capability_lock);
 
      if (!ret && copy_to_user(dataptr, &data, sizeof data))

thanks,
-chris

Signature · 1 line

Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Daniel Jacobowitz

author

2002-10-02 13:17 UTC #4 lore raw

Quoted 6 lines · Chris Wright

Quoted 2 lines · * Daniel Jacobowitz ([email protected]) wrote:
Yes. It was pointed out to me that libcap2 snapshots behave correctly.
Ah, thanks for the info. Hmm, libcap2 still looks like it sets up the header with pid == 0. Maybe I'm missing something.

Look at cap_proc.c:_libcap_cappid :

#if (_LINUX_CAPABILITY_VERSION == 0x19980330)

    if (_libcap_kernel_version == 0x19980330) {
        cap_d->head.pid = pid;
    }
    _cap_debug("pid: %d\n", cap_d->head.pid);

Quoted 4 lines

Quoted 2 lines
Not init: swapper.
Yes, although INIT_TASK sets up the task_struct for swapper.

Oh, right.

Quoted 11 lines

Quoted 5 lines
Try it on 2.4:
drow@nevyn:~% getpcaps 0
Capabilities for `0': =
2.5.40 gives me a very different answer :)
Heh, you're right. However, 2.5.20 behaves the same as 2.4. Looking back this appears to be caused by 2.5.21 locking cleanups done by rml. The older code interpreted pid == 0 to mean current, whereas the new code unconditionally does find_task_by_pid(0). This patch fixes that, and then pid == 0 from libcap should work again.

How very odd. I have been running 2.5 on that machine for a while, and the bug only showed up somewhere between 2.5.36 and 2.5.40. Maybe a coincidence triggered by the PID hashing... your tabbing is a little odd but the patch looks right to me. Thanks!

Quoted 53 lines

kernel/capability.c +9 ~~-10~~

@@ -33,7 +33,7 @@
      int ret = 0;
      pid_t pid;
      __u32 version;
-     task_t *target;
+     task_t *target = current;
      struct __user_cap_data_struct data;
 
      if (get_user(version, &header->version))

@@ -52,21 +52,20 @@
              return -EINVAL;
 
      spin_lock(&task_capability_lock);
-     read_lock(&tasklist_lock); 
 
-     target = find_task_by_pid(pid);
-     if (!target) {
-          ret = -ESRCH;
-          goto out;
+     if (pid && pid != current->pid) {
+     	read_lock(&tasklist_lock); 
+     	target = find_task_by_pid(pid);
+     	read_unlock(&tasklist_lock); 
+     	if (!target) {
+          	ret = -ESRCH;
+          	goto out;
+	}
      }
 
-     data.permitted = cap_t(target->cap_permitted);
-     data.inheritable = cap_t(target->cap_inheritable); 
-     data.effective = cap_t(target->cap_effective);
      ret = security_ops->capget(target, &data.effective, &data.inheritable, &data.permitted);
 
 out:
-     read_unlock(&tasklist_lock); 
      spin_unlock(&task_capability_lock);
 
      if (!ret && copy_to_user(dataptr, &data, sizeof data))

thanks,
-chris

Signature · 6 lines

Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Signature · 2 lines

Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Robert Love

2002-10-02 14:28 UTC #5 lore raw

Quoted 4 lines · Daniel Jacobowitz

How very odd. I have been running 2.5 on that machine for a while, and the bug only showed up somewhere between 2.5.36 and 2.5.40. Maybe a coincidence triggered by the PID hashing... your tabbing is a little odd but the patch looks right to me. Thanks!

I too wonder if the bug is due to the pid changes and not me :)

find_task_by_pid(0) should just return current, which was my intention to avoid conditional code-paths. Maybe I should of used find_process_by_pid()... it seems in the latest 2.5 that still returns current if !pid, at least.

Robert Love

Chris Wright

2002-10-02 16:47 UTC #6 lore raw

Quoted 8 lines · * Robert Love ([email protected]) wrote:

Quoted 4 lines · Daniel Jacobowitz
How very odd. I have been running 2.5 on that machine for a while, and the bug only showed up somewhere between 2.5.36 and 2.5.40. Maybe a coincidence triggered by the PID hashing... your tabbing is a little odd but the patch looks right to me. Thanks!
I too wonder if the bug is due to the pid changes and not me :)

No way man! ;-)

Quoted 4 lines

find_task_by_pid(0) should just return current, which was my intention to avoid conditional code-paths. Maybe I should of used find_process_by_pid()... it seems in the latest 2.5 that still returns current if !pid, at least.

kernel/sched.c::static inline task_t *find_process_by_pid...

Guess that won't work w/out more changes. Perhaps it's simpler/safer to be just be explicit in this case.

thanks,
-chris

Signature · 1 line

Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Robert Love

2002-10-02 18:06 UTC #7 lore raw

Quoted 6 lines · Chris Wright

kernel/sched.c::static inline task_t *find_process_by_pid...
Guess that won't work w/out more changes. Perhaps it's simpler/safer to be just be explicit in this case.
From 2.5.40:

        static inline task_t *find_process_by_pid(pid_t pid)
        {
        	return pid ? find_task_by_pid(pid) : current;
        }

should work :)

Robert Love

Chris Wright

2002-10-02 18:23 UTC #8 lore raw

Quoted 7 lines · * Robert Love ([email protected]) wrote:

Quoted 4 lines · Chris Wright
kernel/sched.c::static inline task_t *find_process_by_pid...
Guess that won't work w/out more changes. Perhaps it's simpler/safer to be just be explicit in this case.

Quoted 8 lines

Quoted 1 line

From 2.5.40:

        static inline task_t *find_process_by_pid(pid_t pid)
        {
        	return pid ? find_task_by_pid(pid) : current;
        }

should work :)

heh, sorry, I meant the static inline part.
cheers,
-chris

Signature · 1 line

Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Chris Wright

2002-10-02 16:27 UTC #9 lore raw

In reply to Daniel Jacobowitz

Quoted 2 lines · * Daniel Jacobowitz ([email protected]) wrote:

Look at cap_proc.c:_libcap_cappid :

Right, but cap_get_proc calls _libcap_cappid with pid of 0. At any rate, I believe pid == 0 is intentional to pick up the current capabilities.

Quoted 4 lines

How very odd. I have been running 2.5 on that machine for a while, and the bug only showed up somewhere between 2.5.36 and 2.5.40. Maybe a coincidence triggered by the PID hashing... your tabbing is a little odd but the patch looks right to me. Thanks!

I tried on various older 2.5 kernels (> 2.5.20) and they returned -ESRCH. I agree, the PID hashing probably started picking up swapper. And yes the tabbing is odd. The file is badly in need of Lindent, as it's mostly space tabbed. I didn't want to hide the patch in whitespace changes ;-)

thanks,
-chris

Signature · 1 line

Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net