Re: [Qemu-devel] [PATCH] linux-user: syscall: Add SO_LINGER for setsockopt

[Laurent Vivier]

Le 08/01/2016 03:03, cheng...@emindsoft.com.cn a écrit :
> From: Chen Gang 
> 
> Just implement it according to the other features implementations.
> 
> Signed-off-by: Chen Gang 
> ---
>  linux-user/syscall.c  | 16 +++-
>  linux-user/syscall_defs.h |  5 +
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index f27148a..9f2c871 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1409,6 +1409,9 @@ static abi_long do_setsockopt(int sockfd, int level, 
> int optname,
>  int val;
>  struct ip_mreqn *ip_mreq;
>  struct ip_mreq_source *ip_mreq_source;
> +struct linger lg;
> +struct target_linger *tlg;
> +
>  
>  switch(level) {
>  case SOL_TCP:
> @@ -1659,7 +1662,19 @@ set_timeout:
>  case TARGET_SO_RCVLOWAT:
>   optname = SO_RCVLOWAT;
>   break;
> -break;
> +case TARGET_SO_LINGER:
> +optname = SO_LINGER;
> +if (optlen != sizeof(struct target_linger)) {
> +return -TARGET_EINVAL;
> +}

OK, this time you're right, there is this check in the kernel too...
and optlen is really the length for setsockopt(), not a pointer.

> +if (!lock_user_struct(VERIFY_READ, tlg, optval_addr, 1)) {
> +return -TARGET_EFAULT;
> +}
> +__get_user(lg.l_onoff, >l_onoff);
> +__get_user(lg.l_linger, >l_linger);
> +unlock_user_struct(tlg, optval_addr, 0);

You can't unlock the structure you're going to use.

> +return get_errno(setsockopt(sockfd, SOL_SOCKET, optname,
> + , sizeof(lg)));

Why do you use "SOL_SOCKET" instead of "level" ?

>  default:
>  goto unimplemented;
>  }
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 9d3c537..5a4d565 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -165,6 +165,11 @@ struct target_ip_mreq_source {
>  uint32_t imr_sourceaddr;
>  };
>  
> +struct target_linger {
> +int l_onoff;/* Linger active*/
> +int l_linger;   /* How long to linger for   */
> +};
> +

Must be "abi_int" to force good alignment for the target.

>  struct target_timeval {
>  abi_long tv_sec;
>  abi_long tv_usec;
> 

Re: [Qemu-devel] [PATCH] linux-user: syscall: Add SO_LINGER for setsockopt

[Laurent Vivier]

Le 08/01/2016 10:45, Chen Gang a écrit :
> 
> On 2016年01月08日 16:38, Laurent Vivier wrote:
>>
>>> +if (!lock_user_struct(VERIFY_READ, tlg, optval_addr, 1)) {
>>> +return -TARGET_EFAULT;
>>> +}
>>> +__get_user(lg.l_onoff, >l_onoff);
>>> +__get_user(lg.l_linger, >l_linger);
>>> +unlock_user_struct(tlg, optval_addr, 0);
>>
>> You can't unlock the structure you're going to use.
>>
> 
> OK, thanks.
> 
> 
>>> +return get_errno(setsockopt(sockfd, SOL_SOCKET, optname,
>>> + , sizeof(lg)));
>>
>> Why do you use "SOL_SOCKET" instead of "level" ?
>>
> 
> At present, level is TARGET_SOL_SOCKET, but we need SOL_SOCKET.

Yes, you're right... so there is a bug in TARGET_SO_BINDTODEVICE which
is using "level" :)

> 
> 
>>> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>>> index 9d3c537..5a4d565 100644
>>> --- a/linux-user/syscall_defs.h
>>> +++ b/linux-user/syscall_defs.h
>>> @@ -165,6 +165,11 @@ struct target_ip_mreq_source {
>>>  uint32_t imr_sourceaddr;
>>>  };
>>>  
>>> +struct target_linger {
>>> +int l_onoff;/* Linger active*/
>>> +int l_linger;   /* How long to linger for   */
>>> +};
>>> +
>>
>> Must be "abi_int" to force good alignment for the target.
>>
> 
> OK, thanks.
> 
> 

Re: [Qemu-devel] [PATCH] linux-user: syscall: Add SO_LINGER for setsockopt

[Chen Gang]

On 2016年01月08日 17:57, Laurent Vivier wrote:
> 
 +return get_errno(setsockopt(sockfd, SOL_SOCKET, optname,
 + , sizeof(lg)));
>>>
>>> Why do you use "SOL_SOCKET" instead of "level" ?
>>>
>>
>> At present, level is TARGET_SOL_SOCKET, but we need SOL_SOCKET.
> 
> Yes, you're right... so there is a bug in TARGET_SO_BINDTODEVICE which
> is using "level" :)
> 

OK, thanks. I shall send patch v2 for it, next week.

-- 
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed

Re: [Qemu-devel] [PATCH] linux-user: syscall: Add SO_LINGER for setsockopt

[Chen Gang]

On 2016年01月08日 16:38, Laurent Vivier wrote:
> 
>> +if (!lock_user_struct(VERIFY_READ, tlg, optval_addr, 1)) {
>> +return -TARGET_EFAULT;
>> +}
>> +__get_user(lg.l_onoff, >l_onoff);
>> +__get_user(lg.l_linger, >l_linger);
>> +unlock_user_struct(tlg, optval_addr, 0);
> 
> You can't unlock the structure you're going to use.
> 

OK, thanks.


>> +return get_errno(setsockopt(sockfd, SOL_SOCKET, optname,
>> + , sizeof(lg)));
> 
> Why do you use "SOL_SOCKET" instead of "level" ?
> 

At present, level is TARGET_SOL_SOCKET, but we need SOL_SOCKET.


>> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>> index 9d3c537..5a4d565 100644
>> --- a/linux-user/syscall_defs.h
>> +++ b/linux-user/syscall_defs.h
>> @@ -165,6 +165,11 @@ struct target_ip_mreq_source {
>>  uint32_t imr_sourceaddr;
>>  };
>>  
>> +struct target_linger {
>> +int l_onoff;/* Linger active*/
>> +int l_linger;   /* How long to linger for   */
>> +};
>> +
> 
> Must be "abi_int" to force good alignment for the target.
> 

OK, thanks.


-- 
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed