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.
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)
Thanks,
Laurent
