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 <luesc...@gmail.com> >>>> Tested-by: Matthias Lüscher <luesc...@gmail.com> >>>> Signed-off-by: Laurent Vivier <laur...@vivier.eu> >>>> --- >>>> >>>> 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. >> >> 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é <phi...@redhat.com> > Tested-by: Philippe Mathieu-Daudé <phi...@redhat.com> > Thank you. Laurent
