diff mbox

linux-user/syscall.c: Add SO_RCVTIMEO and SO_SNDTIMEO for getsockopt

Message ID 1452218349-3793-1-git-send-email-chengang@emindsoft.com.cn
State New
Headers show

Commit Message

Chen Gang Jan. 8, 2016, 1:59 a.m. UTC
From: Chen Gang <gang.chen.5i5j@gmail.com>

Just implement them according to the other features implementations.

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 linux-user/syscall.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

Comments

Laurent Vivier Jan. 8, 2016, 8:25 a.m. UTC | #1
Le 08/01/2016 02:59, chengang@emindsoft.com.cn a écrit :
> From: Chen Gang <gang.chen.5i5j@gmail.com>
> 
> Just implement them according to the other features implementations.
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
>  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, &tv, &lv));
> +            if (ret < 0) {
> +                return ret;
> +            }

 if (len > lv)
    len = lv;

> +            if (copy_to_user_timeval(optval_addr, &tv)) {
> +                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
Chen Gang Jan. 8, 2016, 9:40 a.m. UTC | #2
Firstly, thank you very much for your careful work.

On 2016年01月08日 16:25, Laurent Vivier wrote:
> 
> 
> Le 08/01/2016 02:59, chengang@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, &tv, &lv));
>> +            if (ret < 0) {
>> +                return ret;
>> +            }
> 
>  if (len > lv)
>     len = lv;
> 

OK.

>> +            if (copy_to_user_timeval(optval_addr, &tv)) {
>> +                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 Jan. 11, 2016, 6:31 a.m. UTC | #3
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, &tv, &lv));
>>> +            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, &tv)) {
>>> +                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.
diff mbox

Patch

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, &tv, &lv));
+            if (ret < 0) {
+                return ret;
+            }
+            if (copy_to_user_timeval(optval_addr, &tv)) {
+                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;