Patchwork [v2,2/3] linux-user: make do_setsockopt support SOL_RAW ICMP_FILTER socket option

login
register
mail settings
Submitter Jing Huang
Date July 16, 2012, 10:14 a.m.
Message ID <1342433652-3339-1-git-send-email-jing.huang.pku@gmail.com>
Download mbox | patch
Permalink /patch/171115/
State New
Headers show

Comments

dunrong huang - July 16, 2012, 3:23 a.m.
2012/7/16 Jing Huang <jing.huang.pku@gmail.com>:
> This patch makes do_setsockopt() support SOL_RAW ICMP_FILTER socket option.
>
> Signed-off-by: Jing Huang <jing.huang.pku@gmail.com>
> ---
>  linux-user/syscall.c |   20 ++++++++++++++++++++
>  1 files changed, 20 insertions(+), 0 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 28c8ba5..fc8690d 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -60,6 +60,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
>  #include <netinet/ip.h>
>  #include <netinet/tcp.h>
>  #include <linux/wireless.h>
> +#include <linux/icmp.h>
>  #include "qemu-common.h"
>  #ifdef TARGET_GPROF
>  #include <sys/gmon.h>
> @@ -1442,6 +1443,25 @@ static abi_long do_setsockopt(int sockfd, int level, int optname,
>              goto unimplemented;
>          }
>          break;
> +    case SOL_RAW:
> +        switch (optname) {
> +        case ICMP_FILTER:
> +            /*struct icmp_filter takes an u32 value*/
> +            optname = ICMP_FILTER;
Needless assignment statements. optname already has the value  ICMP_FILTER
> +            if (optlen < sizeof(uint32_t)) {
> +                return -TARGET_EINVAL;
> +            }
> +
> +            if (get_user_u32(val, optval_addr)) {
> +                return -TARGET_EFAULT;
> +            }
> +            ret = get_errno(setsockopt(sockfd, level, optname,
> +                                        (char *)&val, sizeof(val)));
Dont need casting &val to char *.
> +            break;
> +        default:
> +            goto unimplemented;
> +        }
> +        break;
>      case TARGET_SOL_SOCKET:
>          switch (optname) {
>              /* Options with 'int' argument.  */
> --
> 1.7.8.6
>
>
Jing Huang - July 16, 2012, 8:13 a.m.
On Mon, Jul 16, 2012 at 11:23 AM, Dunrong Huang <riegamaths@gmail.com> wrote:
> 2012/7/16 Jing Huang <jing.huang.pku@gmail.com>:
>> This patch makes do_setsockopt() support SOL_RAW ICMP_FILTER socket option.
>>
>> Signed-off-by: Jing Huang <jing.huang.pku@gmail.com>
>> ---
>>  linux-user/syscall.c |   20 ++++++++++++++++++++
>>  1 files changed, 20 insertions(+), 0 deletions(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 28c8ba5..fc8690d 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -60,6 +60,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
>>  #include <netinet/ip.h>
>>  #include <netinet/tcp.h>
>>  #include <linux/wireless.h>
>> +#include <linux/icmp.h>
>>  #include "qemu-common.h"
>>  #ifdef TARGET_GPROF
>>  #include <sys/gmon.h>
>> @@ -1442,6 +1443,25 @@ static abi_long do_setsockopt(int sockfd, int level, int optname,
>>              goto unimplemented;
>>          }
>>          break;
>> +    case SOL_RAW:
>> +        switch (optname) {
>> +        case ICMP_FILTER:
>> +            /*struct icmp_filter takes an u32 value*/
>> +            optname = ICMP_FILTER;
> Needless assignment statements. optname already has the value  ICMP_FILTER

do_setsockopt() lacks level=SOL_RAW and optname=ICMP_FILTER.

The ping in linux-user guest invokes:
setsockopt(icmp_sock, SOL_RAW, ICMP_FILTER, (char *)&filt,
sizeof(filt)) in main() function.


>> +            if (optlen < sizeof(uint32_t)) {
>> +                return -TARGET_EINVAL;
>> +            }
>> +
>> +            if (get_user_u32(val, optval_addr)) {
>> +                return -TARGET_EFAULT;
>> +            }
>> +            ret = get_errno(setsockopt(sockfd, level, optname,
>> +                                        (char *)&val, sizeof(val)));
> Dont need casting &val to char *.

I refer to the definition of setsockopt() in linux-kernel/net/tipc/socket.c

>> +            break;
>> +        default:
>> +            goto unimplemented;
>> +        }
>> +        break;
>>      case TARGET_SOL_SOCKET:
>>          switch (optname) {
>>              /* Options with 'int' argument.  */
>> --
>> 1.7.8.6
>>
>>
>
>
>
> --
> Best Regards,
>
> Dunrong Huang
Jing Huang - July 16, 2012, 10:14 a.m.
This patch makes do_setsockopt() support SOL_RAW ICMP_FILTER socket option.

Signed-off-by: Jing Huang <jing.huang.pku@gmail.com>
---
 linux-user/syscall.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
Peter Maydell - July 16, 2012, 2:36 p.m.
On 16 July 2012 11:14, Jing Huang <jing.huang.pku@gmail.com> wrote:
> This patch makes do_setsockopt() support SOL_RAW ICMP_FILTER socket option.
>
> Signed-off-by: Jing Huang <jing.huang.pku@gmail.com>
> ---
>  linux-user/syscall.c |   20 ++++++++++++++++++++
>  1 files changed, 20 insertions(+), 0 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 28c8ba5..fc8690d 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -60,6 +60,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base,

>  #include <netinet/ip.h>
>  #include <netinet/tcp.h>
>  #include <linux/wireless.h>
> +#include <linux/icmp.h>
>  #include "qemu-common.h"
>  #ifdef TARGET_GPROF
>  #include <sys/gmon.h>
> @@ -1442,6 +1443,25 @@ static abi_long do_setsockopt(int sockfd, int level, int optname,
>              goto unimplemented;
>          }
>          break;
> +    case SOL_RAW:
> +        switch (optname) {
> +        case ICMP_FILTER:
> +            /*struct icmp_filter takes an u32 value*/

Please add spaces after the "/*" and before the "*/".

> +            optname = ICMP_FILTER;

Pointless assignment, as Dunrong says.

> +            if (optlen < sizeof(uint32_t)) {
> +                return -TARGET_EINVAL;
> +            }
> +
> +            if (get_user_u32(val, optval_addr)) {
> +                return -TARGET_EFAULT;
> +            }
> +            ret = get_errno(setsockopt(sockfd, level, optname,
> +                                        (char *)&val, sizeof(val)));

Agreed with Dunrong, this is a pointless cast; setsockopt() takes a
void* for the optval pointer.
(Compare posix spec and see the other calls to setsockopt in this
function. The prototype of the internal kernel function implementing
setsockopt is not relevant because that is not the API that the
kernel exposes to userspace.)

> +            break;
> +        default:
> +            goto unimplemented;
> +        }
> +        break;
>      case TARGET_SOL_SOCKET:
>          switch (optname) {
>              /* Options with 'int' argument.  */
> --
> 1.7.8.6
>

-- PMM

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 28c8ba5..fc8690d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -60,6 +60,7 @@  int __clone2(int (*fn)(void *), void *child_stack_base,
 #include <netinet/ip.h>
 #include <netinet/tcp.h>
 #include <linux/wireless.h>
+#include <linux/icmp.h>
 #include "qemu-common.h"
 #ifdef TARGET_GPROF
 #include <sys/gmon.h>
@@ -1442,6 +1443,25 @@  static abi_long do_setsockopt(int sockfd, int level, int optname,
             goto unimplemented;
         }
         break;
+    case SOL_RAW:
+        switch (optname) {
+        case ICMP_FILTER:
+            /*struct icmp_filter takes an u32 value*/
+            optname = ICMP_FILTER;
+            if (optlen < sizeof(uint32_t)) {
+                return -TARGET_EINVAL;
+            }
+
+            if (get_user_u32(val, optval_addr)) {
+                return -TARGET_EFAULT;
+            }
+            ret = get_errno(setsockopt(sockfd, level, optname,
+                                        (char *)&val, sizeof(val)));
+            break;
+        default:
+            goto unimplemented;
+        }
+        break;
     case TARGET_SOL_SOCKET:
         switch (optname) {
             /* Options with 'int' argument.  */