Patchwork linux-user: Add naive implementation of capget() syscall

login
register
mail settings
Submitter Karol Lewandowski
Date Sept. 19, 2012, 3:09 p.m.
Message ID <1348067345-13633-1-git-send-email-k.lewandowsk@samsung.com>
Download mbox | patch
Permalink /patch/185251/
State New
Headers show

Comments

Karol Lewandowski - Sept. 19, 2012, 3:09 p.m.
libcap, library used to manipulate posix file capabilities uses
getcap() to query version of capabilities supported by running
kernel. Information obtained from this syscall is then used to
initialize data structures that can be used to set process
or/and file capabilities.

Providing capget() alone makes it possible to set posix file
capabilities under qemu (using setcap(8)).

Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
---
 linux-user/syscall.c |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)
Blue Swirl - Sept. 22, 2012, 12:07 p.m.
On Wed, Sep 19, 2012 at 3:09 PM, Karol Lewandowski
<k.lewandowsk@samsung.com> wrote:
> libcap, library used to manipulate posix file capabilities uses
> getcap() to query version of capabilities supported by running
> kernel. Information obtained from this syscall is then used to
> initialize data structures that can be used to set process
> or/and file capabilities.
>
> Providing capget() alone makes it possible to set posix file
> capabilities under qemu (using setcap(8)).
>
> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
> ---
>  linux-user/syscall.c |   20 +++++++++++++++++++-
>  1 files changed, 19 insertions(+), 1 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 6257a04..bcd7a05 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -56,6 +56,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
>  #include <utime.h>
>  #include <sys/sysinfo.h>
>  #include <sys/utsname.h>
> +#include <sys/capability.h>
>  //#include <sys/user.h>
>  #include <netinet/ip.h>
>  #include <netinet/tcp.h>
> @@ -97,6 +98,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
>  #include <linux/fb.h>
>  #include <linux/vt.h>
>  #include <linux/dm-ioctl.h>
> +#include <linux/capability.h>
>  #include "linux_loop.h"
>  #include "cpu-uname.h"
>
> @@ -328,6 +330,11 @@ static int sys_getcwd1(char *buf, size_t size)
>    return strlen(buf)+1;
>  }
>
> +static int sys_capget(struct __user_cap_header_struct *hdr, struct __user_cap_data_struct *data)
> +{
> +  return (capget(hdr, data));

This is not correct. The structure needs to be converted field by
field to host native format, especially endianness.

The parenthesis aren't useful.

> +}
> +
>  #ifdef CONFIG_ATFILE
>  /*
>   * Host system seems to have atfile syscall stubs available.  We
> @@ -7436,7 +7443,18 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>          unlock_user(p, arg1, ret);
>          break;
>      case TARGET_NR_capget:
> -        goto unimplemented;
> +    {
> +       void *p2 = NULL;
> +       if (!(p = lock_user(VERIFY_WRITE, arg1, sizeof(struct __user_cap_header_struct), 0)))
> +           goto efault;

Here and below you also assume that host structure size matches guest.

Missing braces, please read CODING_STYLE and use checkpatch.pl.

> +       if (arg2 && !(p2 = lock_user(VERIFY_WRITE, arg2, sizeof(struct __user_cap_data_struct), 0)))
> +           goto efault;
> +       ret = get_errno(sys_capget(p, p2));
> +       unlock_user(p, arg1, sizeof(struct __user_cap_header_struct));
> +       if (arg2)
> +            unlock_user(p, arg2, sizeof(struct __user_cap_data_struct));
> +       break;
> +    }
>      case TARGET_NR_capset:
>          goto unimplemented;
>      case TARGET_NR_sigaltstack:
> --
> 1.7.5.4
>
>
Erik de Castro Lopo - Sept. 22, 2012, 11:23 p.m.
Blue Swirl wrote:

> This is not correct. The structure needs to be converted field by
> field to host native format, especially endianness.

I'm working in a similar syscall implementation (POSIX timers) and
I'm currently testing it in an debian armhf chroot running on my
x86-64 laptop. After quite a bit of debugging its now working perfectly.

However, both armhf and x86-64 are little endian, so I'd like to
make sure it works on a big endian CPU emulation as well. Unfortunately, 
I can't find one that works (I tried ppc, sparc and mips). They all
seem to have different problems and I can't seem to run anything with
the linux-user emulation.

Erik
Blue Swirl - Sept. 23, 2012, 4:02 p.m.
On Sat, Sep 22, 2012 at 11:23 PM, Erik de Castro Lopo
<mle+tools@mega-nerd.com> wrote:
> Blue Swirl wrote:
>
>> This is not correct. The structure needs to be converted field by
>> field to host native format, especially endianness.
>
> I'm working in a similar syscall implementation (POSIX timers) and
> I'm currently testing it in an debian armhf chroot running on my
> x86-64 laptop. After quite a bit of debugging its now working perfectly.

So the structure happens to be the same for this pair by chance,
probably because neither architecture care much about alignment. The
correct fix is still to do the structure conversion.

>
> However, both armhf and x86-64 are little endian, so I'd like to
> make sure it works on a big endian CPU emulation as well. Unfortunately,
> I can't find one that works (I tried ppc, sparc and mips). They all
> seem to have different problems and I can't seem to run anything with
> the linux-user emulation.
>
> Erik
> --
> ----------------------------------------------------------------------
> Erik de Castro Lopo
> http://www.mega-nerd.com/
>
Peter Maydell - Sept. 24, 2012, 1:47 p.m.
On 23 September 2012 17:02, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Sat, Sep 22, 2012 at 11:23 PM, Erik de Castro Lopo
> <mle+tools@mega-nerd.com> wrote:
>> I'm working in a similar syscall implementation (POSIX timers) and
>> I'm currently testing it in an debian armhf chroot running on my
>> x86-64 laptop. After quite a bit of debugging its now working perfectly.
>
> So the structure happens to be the same for this pair by chance,
> probably because neither architecture care much about alignment. The
> correct fix is still to do the structure conversion.

I think Erik is asking not whether to do the structure conversion,
but how to get a setup that would let you test that it is correct
once you've written it...

-- PMM
Karol Lewandowski - Sept. 25, 2012, 2:09 p.m.
On 09/22/2012 02:07 PM, Blue Swirl wrote:

Hi!

>> +static int sys_capget(struct __user_cap_header_struct *hdr, struct __user_cap_data_struct *data)
>> +{
>> +  return (capget(hdr, data));
> 
> This is not correct. The structure needs to be converted field by
> field to host native format, especially endianness.

Right, that was too naive. :)

> The parenthesis aren't useful.


Will drop those.


>> +}
>> +
>>  #ifdef CONFIG_ATFILE
>>  /*
>>   * Host system seems to have atfile syscall stubs available.  We
>> @@ -7436,7 +7443,18 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>          unlock_user(p, arg1, ret);
>>          break;106.116.62.75
>>      case TARGET_NR_capget:
>> -        goto unimplemented;
>> +    {
>> +       void *p2 = NULL;
>> +       if (!(p = lock_user(VERIFY_WRITE, arg1, sizeof(struct __user_cap_header_struct), 0)))
>> +           goto efault;
> 
> Here and below you also assume that host structure size matches guest.


Hmm, all the fields in these structures are __u32 but one. Will fix
that.

> Missing braces, please read CODING_STYLE and use checkpatch.pl.


Ok, good.

Thanks a lot for review!

Regards,

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 6257a04..bcd7a05 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -56,6 +56,7 @@  int __clone2(int (*fn)(void *), void *child_stack_base,
 #include <utime.h>
 #include <sys/sysinfo.h>
 #include <sys/utsname.h>
+#include <sys/capability.h>
 //#include <sys/user.h>
 #include <netinet/ip.h>
 #include <netinet/tcp.h>
@@ -97,6 +98,7 @@  int __clone2(int (*fn)(void *), void *child_stack_base,
 #include <linux/fb.h>
 #include <linux/vt.h>
 #include <linux/dm-ioctl.h>
+#include <linux/capability.h>
 #include "linux_loop.h"
 #include "cpu-uname.h"
 
@@ -328,6 +330,11 @@  static int sys_getcwd1(char *buf, size_t size)
   return strlen(buf)+1;
 }
 
+static int sys_capget(struct __user_cap_header_struct *hdr, struct __user_cap_data_struct *data)
+{
+  return (capget(hdr, data));
+}
+
 #ifdef CONFIG_ATFILE
 /*
  * Host system seems to have atfile syscall stubs available.  We
@@ -7436,7 +7443,18 @@  abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         unlock_user(p, arg1, ret);
         break;
     case TARGET_NR_capget:
-        goto unimplemented;
+    {
+	void *p2 = NULL;
+	if (!(p = lock_user(VERIFY_WRITE, arg1, sizeof(struct __user_cap_header_struct), 0)))
+	    goto efault;
+	if (arg2 && !(p2 = lock_user(VERIFY_WRITE, arg2, sizeof(struct __user_cap_data_struct), 0)))
+	    goto efault;
+	ret = get_errno(sys_capget(p, p2));
+	unlock_user(p, arg1, sizeof(struct __user_cap_header_struct));
+	if (arg2)
+	     unlock_user(p, arg2, sizeof(struct __user_cap_data_struct));
+	break;
+    }
     case TARGET_NR_capset:
         goto unimplemented;
     case TARGET_NR_sigaltstack: