Message ID | 71ad91e4ee9f41f439086d8f9de60501ad304859.1597129029.git.scw@google.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | fcntl, sockopt, and ioctl options | expand |
Le 11/08/2020 à 09:09, Shu-Chun Weng a écrit : > SOL_UDP manipulate options at UDP level. All six options currently defined > in linux source include/uapi/linux/udp.h take integer values. > > Signed-off-by: Shu-Chun Weng <scw@google.com> > Reviewed-by: Laurent Vivier <laurent@vivier.eu> > --- > v1 -> v2: > Split out SOL_UDP into own patch. > Updated do_print_sockopt(). > > linux-user/strace.c | 6 ++++++ > linux-user/syscall.c | 7 +++++-- > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/linux-user/strace.c b/linux-user/strace.c > index 4fff24b880..854b54a2ad 100644 > --- a/linux-user/strace.c > +++ b/linux-user/strace.c > @@ -7,6 +7,7 @@ > #include <sys/mount.h> > #include <arpa/inet.h> > #include <netinet/tcp.h> > +#include <netinet/udp.h> > #include <linux/if_packet.h> > #include <linux/netlink.h> > #include <sched.h> > @@ -2190,6 +2191,11 @@ static void do_print_sockopt(const char *name, abi_long arg1) > print_raw_param(TARGET_ABI_FMT_ld, optname, 0); > print_pointer(optval, 0); > break; > + case SOL_UDP: > + qemu_log("SOL_UDP,"); > + print_raw_param(TARGET_ABI_FMT_ld, optname, 0); > + print_pointer(optval, 0); > + break; > case SOL_IP: > qemu_log("SOL_IP,"); > print_raw_param(TARGET_ABI_FMT_ld, optname, 0); > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 5645862798..177eec5201 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -53,6 +53,7 @@ > //#include <sys/user.h> > #include <netinet/ip.h> > #include <netinet/tcp.h> > +#include <netinet/udp.h> > #include <linux/wireless.h> > #include <linux/icmp.h> > #include <linux/icmpv6.h> > @@ -1938,7 +1939,8 @@ static abi_long do_setsockopt(int sockfd, int level, int optname, > > switch(level) { > case SOL_TCP: > - /* TCP options all take an 'int' value. */ > + case SOL_UDP: > + /* TCP and UDP options all take an 'int' value. */ > if (optlen < sizeof(uint32_t)) > return -TARGET_EINVAL; > > @@ -2586,7 +2588,8 @@ get_timeout: > } > break; > case SOL_TCP: > - /* TCP options all take an 'int' value. */ > + case SOL_UDP: > + /* TCP and UDP options all take an 'int' value. */ > int_case: > if (get_user_u32(len, optlen)) > return -TARGET_EFAULT; > Reviewed-by: Laurent Vivier <laurent@vivier.eu> I'm wondering if the int_case of getsockopt() manages correctly the length: length can be between 0 and sizeof(int), but the int_case only uses a put_user_u32() or a put_user_u8(). Do we need the put_user_u16()? Thanks, Laurent
It does look like something that can be improved. The lines have been there for 14 years though: https://github.com/qemu/qemu/commit/53a5960aadd542dd27b8705ac30df154557d5ffc The potential bug is triggered when the user passes in a 2-byte integer in getsockopt(), which seems uncommon -- do we have guest architectures that use 16-bit int type? Shu-Chun On Tue, Aug 11, 2020 at 7:21 AM Laurent Vivier <laurent@vivier.eu> wrote: > Le 11/08/2020 à 09:09, Shu-Chun Weng a écrit : > > SOL_UDP manipulate options at UDP level. All six options currently > defined > > in linux source include/uapi/linux/udp.h take integer values. > > > > Signed-off-by: Shu-Chun Weng <scw@google.com> > > Reviewed-by: Laurent Vivier <laurent@vivier.eu> > > --- > > v1 -> v2: > > Split out SOL_UDP into own patch. > > Updated do_print_sockopt(). > > > > linux-user/strace.c | 6 ++++++ > > linux-user/syscall.c | 7 +++++-- > > 2 files changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/linux-user/strace.c b/linux-user/strace.c > > index 4fff24b880..854b54a2ad 100644 > > --- a/linux-user/strace.c > > +++ b/linux-user/strace.c > > @@ -7,6 +7,7 @@ > > #include <sys/mount.h> > > #include <arpa/inet.h> > > #include <netinet/tcp.h> > > +#include <netinet/udp.h> > > #include <linux/if_packet.h> > > #include <linux/netlink.h> > > #include <sched.h> > > @@ -2190,6 +2191,11 @@ static void do_print_sockopt(const char *name, > abi_long arg1) > > print_raw_param(TARGET_ABI_FMT_ld, optname, 0); > > print_pointer(optval, 0); > > break; > > + case SOL_UDP: > > + qemu_log("SOL_UDP,"); > > + print_raw_param(TARGET_ABI_FMT_ld, optname, 0); > > + print_pointer(optval, 0); > > + break; > > case SOL_IP: > > qemu_log("SOL_IP,"); > > print_raw_param(TARGET_ABI_FMT_ld, optname, 0); > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > > index 5645862798 <(564)%20586-2798>..177eec5201 100644 > > --- a/linux-user/syscall.c > > +++ b/linux-user/syscall.c > > @@ -53,6 +53,7 @@ > > //#include <sys/user.h> > > #include <netinet/ip.h> > > #include <netinet/tcp.h> > > +#include <netinet/udp.h> > > #include <linux/wireless.h> > > #include <linux/icmp.h> > > #include <linux/icmpv6.h> > > @@ -1938,7 +1939,8 @@ static abi_long do_setsockopt(int sockfd, int > level, int optname, > > > > switch(level) { > > case SOL_TCP: > > - /* TCP options all take an 'int' value. */ > > + case SOL_UDP: > > + /* TCP and UDP options all take an 'int' value. */ > > if (optlen < sizeof(uint32_t)) > > return -TARGET_EINVAL; > > > > @@ -2586,7 +2588,8 @@ get_timeout: > > } > > break; > > case SOL_TCP: > > - /* TCP options all take an 'int' value. */ > > + case SOL_UDP: > > + /* TCP and UDP options all take an 'int' value. */ > > int_case: > > if (get_user_u32(len, optlen)) > > return -TARGET_EFAULT; > > > > Reviewed-by: Laurent Vivier <laurent@vivier.eu> > > I'm wondering if the int_case of getsockopt() manages correctly the > length: length can be between 0 and sizeof(int), but the int_case only > uses a put_user_u32() or a put_user_u8(). Do we need the put_user_u16()? > > Thanks, > Laurent >
diff --git a/linux-user/strace.c b/linux-user/strace.c index 4fff24b880..854b54a2ad 100644 --- a/linux-user/strace.c +++ b/linux-user/strace.c @@ -7,6 +7,7 @@ #include <sys/mount.h> #include <arpa/inet.h> #include <netinet/tcp.h> +#include <netinet/udp.h> #include <linux/if_packet.h> #include <linux/netlink.h> #include <sched.h> @@ -2190,6 +2191,11 @@ static void do_print_sockopt(const char *name, abi_long arg1) print_raw_param(TARGET_ABI_FMT_ld, optname, 0); print_pointer(optval, 0); break; + case SOL_UDP: + qemu_log("SOL_UDP,"); + print_raw_param(TARGET_ABI_FMT_ld, optname, 0); + print_pointer(optval, 0); + break; case SOL_IP: qemu_log("SOL_IP,"); print_raw_param(TARGET_ABI_FMT_ld, optname, 0); diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 5645862798..177eec5201 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -53,6 +53,7 @@ //#include <sys/user.h> #include <netinet/ip.h> #include <netinet/tcp.h> +#include <netinet/udp.h> #include <linux/wireless.h> #include <linux/icmp.h> #include <linux/icmpv6.h> @@ -1938,7 +1939,8 @@ static abi_long do_setsockopt(int sockfd, int level, int optname, switch(level) { case SOL_TCP: - /* TCP options all take an 'int' value. */ + case SOL_UDP: + /* TCP and UDP options all take an 'int' value. */ if (optlen < sizeof(uint32_t)) return -TARGET_EINVAL; @@ -2586,7 +2588,8 @@ get_timeout: } break; case SOL_TCP: - /* TCP options all take an 'int' value. */ + case SOL_UDP: + /* TCP and UDP options all take an 'int' value. */ int_case: if (get_user_u32(len, optlen)) return -TARGET_EFAULT;