qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] linux-user: implement TARGET_SO_PEERSEC


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v2] linux-user: implement TARGET_SO_PEERSEC
Date: Wed, 12 Feb 2020 17:46:51 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1

On 2/12/20 5:43 PM, Laurent Vivier wrote:
Le 12/02/2020 à 17:08, Philippe Mathieu-Daudé a écrit :
On 2/12/20 5:03 PM, Laurent Vivier wrote:
Le 12/02/2020 à 16:56, Philippe Mathieu-Daudé a écrit :
On 2/4/20 10:19 PM, Laurent Vivier wrote:
"The purpose of this option is to allow an application to obtain the
security credentials of a Unix stream socket peer.  It is analogous to
SO_PEERCRED (which provides authentication using standard Unix
credentials
of pid, uid and gid), and extends this concept to other security
models." -- https://lwn.net/Articles/62370/

Until now it was passed to the kernel with an "int" argument and
fails when it was supported by the host because the parameter is
like a filename: it is always a \0-terminated string with no embedded
\0 characters, but is not guaranteed to be ASCII or UTF-8.

I've tested the option with the following program:

       /*
        * cc -o getpeercon getpeercon.c
        */

       #include <stdio.h>
       #include <sys/types.h>
       #include <sys/socket.h>
       #include <netinet/in.h>
       #include <arpa/inet.h>

       int main(void)
       {
           int fd;
           struct sockaddr_in server, addr;
           int ret;
           socklen_t len;
           char buf[256];

           fd = socket(PF_INET, SOCK_STREAM, 0);
           if (fd == -1) {
               perror("socket");
               return 1;
           }

           server.sin_family = AF_INET;
           inet_aton("127.0.0.1", &server.sin_addr);
           server.sin_port = htons(40390);

           connect(fd, (struct sockaddr*)&server, sizeof(server));

           len = sizeof(buf);
           ret = getsockopt(fd, SOL_SOCKET, SO_PEERSEC, buf, &len);
           if (ret == -1) {
               perror("getsockopt");
               return 1;
           }
           printf("%d %s\n", len, buf);
           return 0;
       }

On host:

     $ ./getpeercon
     33 system_u:object_r:unlabeled_t:s0

With qemu-aarch64/bionic without the patch:

     $ ./getpeercon
     getsockopt: Numerical result out of range

With the patch:

     $ ./getpeercon
     33 system_u:object_r:unlabeled_t:s0

Bug: https://bugs.launchpad.net/qemu/+bug/1823790
Reported-by: Matthias Lüscher <address@hidden>
Tested-by: Matthias Lüscher <address@hidden>
Signed-off-by: Laurent Vivier <address@hidden>
---

Notes:
       v2: use correct length in unlock_user()

    linux-user/syscall.c | 22 ++++++++++++++++++++++
    1 file changed, 22 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d60142f0691c..c930577686da 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2344,6 +2344,28 @@ static abi_long do_getsockopt(int sockfd, int
level, int optname,
                }
                break;
            }
+        case TARGET_SO_PEERSEC: {
+            char *name;
+
+            if (get_user_u32(len, optlen)) {
+                return -TARGET_EFAULT;
+            }
+            if (len < 0) {
+                return -TARGET_EINVAL;
+            }
+            name = lock_user(VERIFY_WRITE, optval_addr, len, 0);
+            if (!name) {
+                return -TARGET_EFAULT;
+            }
+            lv = len;
+            ret = get_errno(getsockopt(sockfd, level, SO_PEERSEC,
+                                       name, &lv));

Can we get lv > len?

No:

getsockopt(2)

"For  getsockopt(), optlen is a value-result argument, initially
containing the size of the buffer pointed to by optval, and modified on
return to  indicate the  actual  size  of  the value returned."


+            if (put_user_u32(lv, optlen)) {
+                ret = -TARGET_EFAULT;
+            }
+            unlock_user(name, optval_addr, lv);

Maybe safer to use len instead of lv here?

No:

this is the length of the buffer we must copy back to the user. Kernel
has only modified lv length, not len.

So we can simplify the TARGET_SO_LINGER case then.

No, this case is different because lglen is sizeof(struct linger) and it
can differ from len. So lglen can be greater than len.

If you check the kernel you can see if the buffer is not big enough the
data are partially copied. This is partially done in our code because
the __put_user() can overflow the user memory but we return len to the
caller. To fix that, we should use a local target_linger to change
endianness and then copy the local copy to the user copy using len.

Ah OK I understand now, thanks for the explanation.



linux-user/qemu.h

/* Unlock an area of guest memory.  The first LEN bytes must be
     flushed back to guest memory. host_ptr = NULL is explicitly
     allowed and does nothing. */
static inline void unlock_user(void *host_ptr, abi_ulong guest_addr,
                                 long len)


Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
Tested-by: Philippe Mathieu-Daudé <address@hidden>


Thank you.

Laurent





reply via email to

[Prev in Thread] Current Thread [Next in Thread]