Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Add SO_RCVTIMEO and SO_SNDTIMEO for getsockopt

[Chen Gang]

Oh, sorry, after check again, I guess, we need continue to discuss.

On 2016年01月08日 17:40, Chen Gang wrote:
> 
> On 2016年01月08日 16:25, Laurent Vivier wrote:
>>
>>> +if (optlen < sizeof(struct target_timeval)) {
>>> +return -TARGET_EINVAL;
>>> +}
>>
>> You don't have to check the len (kernel doesn't), EINVAL is not listed
>> in the getsockopt() error cases, it should be an EFAULT, and this will
>> be managed by copy_to_user_timeval().
>>
 

After "man getsockopt", there is EINVAL in its error cases.

> OK.
> 
>>> +lv = sizeof(tv);
>>> +ret = get_errno(getsockopt(sockfd, level, optname, , ));
>>> +if (ret < 0) {
>>> +return ret;
>>> +}
>>
>>  if (len > lv)
>> len = lv;
>>

For me, len is for target, lv is for host, they cann't be compared with
each other.

> 
> OK.
> 
>>> +if (copy_to_user_timeval(optval_addr, )) {
>>> +return -TARGET_EFAULT;
>>> +}
>>> +if (put_user_u32(sizeof(struct target_timeval), optlen)) {
>>> +return -TARGET_EFAULT;
>>> +}
>>
>> if (put_user_u32(len, optlen))
>> return -TARGET_EFAULT;
>>

For me, we still need put sizeof(struct target_timeval) to optlen, since
the host lengh should be sizeof(struct timeval).

> 
> OK. I shall send patch v2 for it, in the next week.
> 
> 

Thanks.
-- 
Chen Gang (陈刚)

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

Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Add SO_RCVTIMEO and SO_SNDTIMEO for getsockopt

[Laurent Vivier]

Le 08/01/2016 02:59, cheng...@emindsoft.com.cn a écrit :
> From: Chen Gang 
> 
> Just implement them according to the other features implementations.
> 
> Signed-off-by: Chen Gang 
> ---
>  linux-user/syscall.c | 25 +++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 12a6cd2..f27148a 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1685,6 +1685,7 @@ static abi_long do_getsockopt(int sockfd, int level, 
> int optname,
>  abi_long ret;
>  int len, val;
>  socklen_t lv;
> +struct timeval tv;
>  
>  switch(level) {
>  case TARGET_SOL_SOCKET:
> @@ -1692,10 +1693,30 @@ static abi_long do_getsockopt(int sockfd, int level, 
> int optname,
>  switch (optname) {
>  /* These don't just return a single integer */
>  case TARGET_SO_LINGER:
> -case TARGET_SO_RCVTIMEO:
> -case TARGET_SO_SNDTIMEO:
>  case TARGET_SO_PEERNAME:
>  goto unimplemented;
> +

useless blank line

> +case TARGET_SO_RCVTIMEO:
> +optname = SO_RCVTIMEO;
> +goto time_case;
> +case TARGET_SO_SNDTIMEO:
> +optname = SO_SNDTIMEO;
> +time_case:

Something like in "int_case", I think optlen is a pointer, not the length:

if (get_user_u32(len, optlen))
return -TARGET_EFAULT;
if (len < 0)
return -TARGET_EINVAL;


> +if (optlen < sizeof(struct target_timeval)) {
> +return -TARGET_EINVAL;
> +}

You don't have to check the len (kernel doesn't), EINVAL is not listed
in the getsockopt() error cases, it should be an EFAULT, and this will
be managed by copy_to_user_timeval().

> +lv = sizeof(tv);
> +ret = get_errno(getsockopt(sockfd, level, optname, , ));
> +if (ret < 0) {
> +return ret;
> +}

 if (len > lv)
len = lv;

> +if (copy_to_user_timeval(optval_addr, )) {
> +return -TARGET_EFAULT;
> +}
> +if (put_user_u32(sizeof(struct target_timeval), optlen)) {
> +return -TARGET_EFAULT;
> +}

if (put_user_u32(len, optlen))
return -TARGET_EFAULT;


> +break;
>  case TARGET_SO_PEERCRED: {
>  struct ucred cr;
>  socklen_t crlen;
> 

Laurent

Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Add SO_RCVTIMEO and SO_SNDTIMEO for getsockopt

[Chen Gang]

Firstly, thank you very much for your careful work.

On 2016年01月08日 16:25, Laurent Vivier wrote:
> 
> 
> Le 08/01/2016 02:59, cheng...@emindsoft.com.cn a écrit :
[...]
>> @@ -1692,10 +1693,30 @@ static abi_long do_getsockopt(int sockfd, int level, 
>> int optname,
>>  switch (optname) {
>>  /* These don't just return a single integer */
>>  case TARGET_SO_LINGER:
>> -case TARGET_SO_RCVTIMEO:
>> -case TARGET_SO_SNDTIMEO:
>>  case TARGET_SO_PEERNAME:
>>  goto unimplemented;
>> +
> 
> useless blank line
> 

OK.

>> +case TARGET_SO_RCVTIMEO:
>> +optname = SO_RCVTIMEO;
>> +goto time_case;
>> +case TARGET_SO_SNDTIMEO:
>> +optname = SO_SNDTIMEO;
>> +time_case:
> 
> Something like in "int_case", I think optlen is a pointer, not the length:
> 
> if (get_user_u32(len, optlen))
> return -TARGET_EFAULT;
> if (len < 0)
> return -TARGET_EINVAL;
> 

OK.

> 
>> +if (optlen < sizeof(struct target_timeval)) {
>> +return -TARGET_EINVAL;
>> +}
> 
> You don't have to check the len (kernel doesn't), EINVAL is not listed
> in the getsockopt() error cases, it should be an EFAULT, and this will
> be managed by copy_to_user_timeval().
> 

OK.

>> +lv = sizeof(tv);
>> +ret = get_errno(getsockopt(sockfd, level, optname, , ));
>> +if (ret < 0) {
>> +return ret;
>> +}
> 
>  if (len > lv)
> len = lv;
> 

OK.

>> +if (copy_to_user_timeval(optval_addr, )) {
>> +return -TARGET_EFAULT;
>> +}
>> +if (put_user_u32(sizeof(struct target_timeval), optlen)) {
>> +return -TARGET_EFAULT;
>> +}
> 
> if (put_user_u32(len, optlen))
> return -TARGET_EFAULT;
> 

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


Thanks.
-- 
Chen Gang (陈刚)

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

[Qemu-devel] [PATCH] linux-user/syscall.c: Add SO_RCVTIMEO and SO_SNDTIMEO for getsockopt

[chengang]

From: Chen Gang 

Just implement them according to the other features implementations.

Signed-off-by: Chen Gang 
---
 linux-user/syscall.c | 25 +++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 12a6cd2..f27148a 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1685,6 +1685,7 @@ static abi_long do_getsockopt(int sockfd, int level, int 
optname,
 abi_long ret;
 int len, val;
 socklen_t lv;
+struct timeval tv;
 
 switch(level) {
 case TARGET_SOL_SOCKET:
@@ -1692,10 +1693,30 @@ static abi_long do_getsockopt(int sockfd, int level, 
int optname,
 switch (optname) {
 /* These don't just return a single integer */
 case TARGET_SO_LINGER:
-case TARGET_SO_RCVTIMEO:
-case TARGET_SO_SNDTIMEO:
 case TARGET_SO_PEERNAME:
 goto unimplemented;
+
+case TARGET_SO_RCVTIMEO:
+optname = SO_RCVTIMEO;
+goto time_case;
+case TARGET_SO_SNDTIMEO:
+optname = SO_SNDTIMEO;
+time_case:
+if (optlen < sizeof(struct target_timeval)) {
+return -TARGET_EINVAL;
+}
+lv = sizeof(tv);
+ret = get_errno(getsockopt(sockfd, level, optname, , ));
+if (ret < 0) {
+return ret;
+}
+if (copy_to_user_timeval(optval_addr, )) {
+return -TARGET_EFAULT;
+}
+if (put_user_u32(sizeof(struct target_timeval), optlen)) {
+return -TARGET_EFAULT;
+}
+break;
 case TARGET_SO_PEERCRED: {
 struct ucred cr;
 socklen_t crlen;
-- 
1.9.1