Re: [PATCH] Re: Capabilities-related change in 2.5.40

[Daniel Jacobowitz <dan@debian.org>]

On Wed, Oct 02, 2002 at 12:38:17AM -0700, Chris Wright wrote:
> * Daniel Jacobowitz (dan@debian.org) 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);

> 
> > Not init: swapper.
> 
> Yes, although INIT_TASK sets up the task_struct for swapper.

Oh, right.

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

> 
> --- 1.5/kernel/capability.c	Sun Sep 15 12:19:29 2002
> +++ edited/kernel/capability.c	Wed Oct  2 00:28:32 2002
> @@ -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
> -- 
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer