Bug 447518

Created attachment 306100 [details]
Example program

Actually, make that 12 bytes. I'll attach another program that outputs this:

0000000000000000000000004141414141414141

This is a hex output of the buffer that was filled with 'A's before capget() call.

Created attachment 306104 [details]
A program to verify how much of a buffer was used

Why has this been reassigned to glibc?  catget in glibc is just a syscall
wrapper, with no extra code whatsoever, and sys/capability.h includes
linux/capability.h
from kernel-headers to get the types.
So how much data is filled in and what are the structure sizes is solely a
kernel issue.

Your program compiles and runs without the crash on x86_64.

It's been filed for i686.

Just note that in order to see the crash, the first program needs to modified to
remove extra 10 bytes being allocated. I verified on another i686 machine. I'll
attach that program as well.

Created attachment 306360 [details]
Program that creates the crash

It seems to me that this sample code is broken.

The assumption of the provided sample code is that is understands what
the data format is for the prevailing kernel magic:

=========================
#include <stdio.h>
#include <stdlib.h>
#include <sys/capability.h>

int main(int argc,char **argv){
  cap_user_header_t head=malloc(sizeof(*head));
  cap_user_data_t data=malloc(sizeof(*data));

  head->version=_LINUX_CAPABILITY_VERSION;

  capget(head,data);

  free(data);
  free(head);

  return 0;
}
=========================

Unfortunately, it is not interpreting the value of
_LINUX_CAPABILITY_VERSION - which does, after all, version the
size/meaning of the 'head' and 'data'. If the user wants to supply a
buffer of this size, then they should use

   head->version = _LINUX_CAPABILITY_VERSION_1

on the other hand, the user might want to explore using libcap to
protect him from this level of detail.

So, what you saying is that Squid (see bug 447045) actually uses this
incorrectly, right? They should be either passing in _LINUX_CAPABILITY_VERSION_1
or doing POSIX calls cap_get_proc()/cap_set_proc().

Please confirm so that I can close and notify Squid folks.

Ah, sorry. Spoke before seeing e-mails from other kernel folks. Please disregard
comment #10.

Just of of curiosity, if _LINUX_CAPABILITY_VERSION is chosen (as opposed to
_LINUX_CAPABILITY_VERSION_1 as you suggested), with what size buffers should
capget() be called?

(In reply to comment #12)
> Just of of curiosity, if _LINUX_CAPABILITY_VERSION is chosen (as opposed to
> _LINUX_CAPABILITY_VERSION_1 as you suggested), with what size buffers should
> capget() be called?

The kernel headers won't actually let you use VERSION_2 afaict. Looks like old
programs compiled with old headers should be fine, but maybe the default in the
kernel headers should be version 1 since that's the only one that will work right.

Created attachment 306503 [details]
Proposed kernel (include/linux/capability.h) 'fix' for this issue

I'm not sure I follow comment #13, version's 1 and 2 work fine for me.

libcap2, for example, does something like the following to figure out which ABI
the running kernel prefers/supports:

   struct __user_cap_header_struct head;
   head.version = _LINUX_CAPABILITY_VERSION;
   (void) capget(&head, NULL);
   switch (head.version) {
   case _LINUX_CAPABILITY_VERSION_1:
      version = _LINUX_CAPABILITY_VERSION_1;
      u32s = _LINUX_CAPABILITY_U32S_1;
      break;
   case _LINUX_CAPABILITY_VERSION_2:
      version = _LINUX_CAPABILITY_VERSION_2;
      u32s = _LINUX_CAPABILITY_U32S_2;
      break;
   default:
       abort("no idea what to do"); /* new version not defined */
   }

   data = calloc(u32s, sizeof(struct __user_cap_data_struct));
   head.pid = pid_of_target;
   capget(&head, data);

If the kernel evolves a third iteration of the ABI, then this switch etc code
will evolve again.

In the light of this bug report, libcap will likely stop using
_LINUX_CAPABILITY_VERSION at all and be more explicit with version numbers,
since I expect the _LINUX_CAPABILITY_VERSION value to revert to
_LINUX_CAPABILITY_VERSION_1 to help suppress issues like the one raised in this
bug report.

So, thanks for highlighting this issue in general!

(In reply to comment #13)
> 
> I'm not sure I follow comment #13, version's 1 and 2 work fine for me.
> 

You can't use version 2 unless you write your own headers; the kernel headers
don't provide any memory structures for using it even though they set the
default to 2.

Created attachment 306879 [details]
new kernel patch from lkml

This patch looks even better...

(In reply to comment #15)
> (In reply to comment #13)
> > 
> > I'm not sure I follow comment #13, version's 1 and 2 work fine for me.
> > 
> 
> You can't use version 2 unless you write your own headers; the kernel headers
> don't provide any memory structures for using it even though they set the
> default to 2.
> 

  typedef struct __user_cap_data_struct {
        __u32 effective;
        __u32 permitted;
        __u32 inheritable;
  } __user *cap_user_data_t;

this type is unchanged. With v2(and now v3), the pointer data refers to

  struct __user_cap_data_struct data[_LINUX_CAPABILITY_U32S_3];

Hope that helps.

PS. If we ever need v4 capabilities, we reserve the right to redefine this data
structure...

Hope that helps.

Andrew

Patch went in 2.6.25.4-47

kernel-2.6.25.6-55.fc9 has been submitted as an update for Fedora 9

kernel-2.6.25.6-55.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.