Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
Paolo Bonzini, on jeu. 04 janv. 2018 18:57:18 +0100, wrote: > On 04/01/2018 18:45, Samuel Thibault wrote: > > Paolo Bonzini, on jeu. 04 janv. 2018 18:11:00 +0100, wrote: > >> On 04/01/2018 16:56, Samuel Thibault wrote: > >>>> However, adding magic to "-device usb-braille" that creates both a > >>>> front-end and a back-end is completely the opposite of sane... > >>> Well, this is also what happens with -device usb-mouse, usb-kbd etc.: > >>> they also plug with keyboard & mouse pipes of the qemu graphical > >>> frontend. braille is just the same vein for the user. > >> > >> No, they don't create a new UI object magically that wasn't there > >> before. They just let you add a device, e.g. a USB tablet, that listens > >> on an _existing_ UI (GTK+ or SDL or similar). > > > > Technically for the qemu developper, yes. > > > > But for the user, no. > > That's not true. With or without "-usbdevice tablet", you use the same > mouse or keyboard device as the input source. Yes, because it preexists in the mind of the user. That's the same for braille. The fact that braille has to go through a character-to-brlapi conversion is not the business of the user, he just wants braille out. Samuel
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
Paolo Bonzini, on jeu. 04 janv. 2018 18:11:00 +0100, wrote: > On 04/01/2018 16:56, Samuel Thibault wrote: > >> However, adding magic to "-device usb-braille" that creates both a > >> front-end and a back-end is completely the opposite of sane... > > Well, this is also what happens with -device usb-mouse, usb-kbd etc.: > > they also plug with keyboard & mouse pipes of the qemu graphical > > frontend. braille is just the same vein for the user. > > No, they don't create a new UI object magically that wasn't there > before. They just let you add a device, e.g. a USB tablet, that listens > on an _existing_ UI (GTK+ or SDL or similar). Technically for the qemu developper, yes. But for the user, no. Samuel
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
Paolo Bonzini, on jeu. 04 janv. 2018 16:47:16 +0100, wrote: > The point of deprecation is not to make the interface simpler, it is to > avoid cases where one option is doing too much and/or crossing > abstraction boundaries, for example -net creating both a device and a > hub port. > > "-serial" is okay because it only creates the front-end, it's the board > that decides how to attach it to the back-end. > > -usbdevice creating both a front-end and a back-end is not a problem per > se. The issue with -usbdevice is just that it's too much code. > > However, adding magic to "-device usb-braille" that creates both a > front-end and a back-end is completely the opposite of sane... Well, this is also what happens with -device usb-mouse, usb-kbd etc.: they also plug with keyboard & mouse pipes of the qemu graphical frontend. braille is just the same vein for the user. Samuel
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
Paolo Bonzini, on jeu. 04 janv. 2018 16:28:27 +0100, wrote: > On 04/01/2018 16:24, Samuel Thibault wrote: > >> Maybe we can add "-braille" instead. > > I guess such legacy-looking option would be frowned up :) > > Not really. As long as it doesn't propagate everywhere in device code, > it's fine. We are not going to deprecate "-serial mon:stdio", so... > > > And anyway we need to be able to specify either -serial braille or > > -device usb-braille. > > So "-braille serial" and "-braille usb"? Well, why not, it just doesn't seem to me much simpler than "-serial braille" and "-device usb-braille" :) Samuel
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
Hello, Paolo Bonzini, on jeu. 04 janv. 2018 16:19:02 +0100, wrote: > On 04/01/2018 15:21, Samuel Thibault wrote: > > Samuel Thibault, on jeu. 04 janv. 2018 15:15:24 +0100, wrote: > >> I'm however still unable to make usb-serial and usb-braille work, as > >> mentioned in my other mail :) > > > > Ah, now with the documentation I understand, one has to also pass > > -chardev braille,chardev=foobar, and then > > -device usb-braille,chardev=foobar works. > > > > Would it be possible to make this automatic? Before I could tell people > > "use -usbdevice braille to test braille", and now it would be > > "use -chardev braille,chardev=foobar -device usb-braille,chardev=foobar" > > Maybe we can add "-braille" instead. I guess such legacy-looking option would be frowned up :) And anyway we need to be able to specify either -serial braille or -device usb-braille. Samuel
Re: [Qemu-devel] [PATCH] usb: Remove legacy -usbdevice option
Hello, Thomas Huth, on jeu. 04 janv. 2018 16:01:24 +0100, wrote: > On 04.01.2018 15:23, Samuel Thibault wrote: > > Thomas Huth, on jeu. 04 janv. 2018 15:22:25 +0100, wrote: > >> On 04.01.2018 14:26, Samuel Thibault wrote: > >> I think it's more common to not do any magic default setup with > >> "-device", but if you think this should be the case here, it can > >> certainly be added again > > > > Yes, please: it will almost never be used differently than with just a > > fresh braille chardev. > > It's pretty much untested, but would a patch like this help? Completely! -usb -device usb-braille just work fine with it :) Thanks, Samuel
Re: [Qemu-devel] [PATCH] usb: Remove legacy -usbdevice option
Thomas Huth, on jeu. 04 janv. 2018 15:22:25 +0100, wrote: > On 04.01.2018 14:26, Samuel Thibault wrote: > I think it's more common to not do any magic default setup with > "-device", but if you think this should be the case here, it can > certainly be added again Yes, please: it will almost never be used differently than with just a fresh braille chardev. Samuel
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
Samuel Thibault, on jeu. 04 janv. 2018 15:15:24 +0100, wrote: > I'm however still unable to make usb-serial and usb-braille work, as > mentioned in my other mail :) Ah, now with the documentation I understand, one has to also pass -chardev braille,chardev=foobar, and then -device usb-braille,chardev=foobar works. Would it be possible to make this automatic? Before I could tell people "use -usbdevice braille to test braille", and now it would be "use -chardev braille,chardev=foobar -device usb-braille,chardev=foobar" ... Samuel
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
Thomas Huth, on jeu. 04 janv. 2018 15:12:56 +0100, wrote:
> On 04.01.2018 15:09, Samuel Thibault wrote:
> > Thomas Huth, on jeu. 04 janv. 2018 14:15:52 +0100, wrote:
> >> -@item host:@var{bus}.@var{addr}
> >> -Pass through the host device identified by @var{bus}.@var{addr} (Linux
> >> only).
> >> -
> >> -@item host:@var{vendor_id}:@var{product_id}
> >> -Pass through the host device identified by
> >> @var{vendor_id}:@var{product_id}
> >> -(Linux only).
> >> -
> >> -@item
> >> serial:[vendorid=@var{vendor_id}][,productid=@var{product_id}]:@var{dev}
> >> -Serial converter to host character device @var{dev}, see @code{-serial}
> >> for the
> >> -available devices.
> >
> > as mentioned in my other mail, I don't find any place in the manpage
> > where that is now documented. I believe this -usbdevice foo
> > documentation needs to be converted to -device usb-foo instead of
> > removed. Otherwise as it is now there is no way to even know that
> > -device usb-mouse, -device usb-disk, etc. are available at all.
>
> I already did convert the chapter that describes the USB devices a
> couple of months ago, you can find it in our qemu-doc.html here:
>
> https://qemu.weilnetz.de/doc/qemu-doc.html#usb_005fdevices
Ah, ok, thanks.
I'm however still unable to make usb-serial and usb-braille work, as
mentioned in my other mail :)
Samuel
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
Thomas Huth, on jeu. 04 janv. 2018 14:15:52 +0100, wrote:
> -@item host:@var{bus}.@var{addr}
> -Pass through the host device identified by @var{bus}.@var{addr} (Linux only).
> -
> -@item host:@var{vendor_id}:@var{product_id}
> -Pass through the host device identified by @var{vendor_id}:@var{product_id}
> -(Linux only).
> -
> -@item
> serial:[vendorid=@var{vendor_id}][,productid=@var{product_id}]:@var{dev}
> -Serial converter to host character device @var{dev}, see @code{-serial} for
> the
> -available devices.
as mentioned in my other mail, I don't find any place in the manpage
where that is now documented. I believe this -usbdevice foo
documentation needs to be converted to -device usb-foo instead of
removed. Otherwise as it is now there is no way to even know that
-device usb-mouse, -device usb-disk, etc. are available at all.
Samuel
Re: [Qemu-devel] [PATCH] usb: Remove legacy -usbdevice option
Hello, I'm afraid I can't even work out how to replace -usbdevice braille (or -usbdevice serial, all questions below apply, except making chardev default to braille): ... -usbdevice braille -usbdevice braille: '-usbdevice' is deprecated, please use '-device usb-...' instead but -device usb-serial is not actually documented in the manual page (same for usb-keyboard, usb-tablet, etc. for instance I can't find in the manpage anything about replacement for -usbdevice host::) ... -device usb-braille -device usb-braille: No 'usb-bus' bus found for device 'usb-braille' ... -device usb-bus -device usb-braille -device usb-bus: 'usb-bus' is not a valid device model name So the error message above should suggest -usb. But then ... -usb -device usb-braille -device usb-braille: Property chardev is required that used to be a default, could we keep it a default when using -device? Hardly anybody would want to use usb-braille with something else than chardev braille. ... -usb -device usb-braille,chardev=braille -device usb-serial,chardev=braille: Property 'usb-serial.chardev' can't find value 'braille' Uh? It seems there is a parsing issue here? So I'm afraid a lot is still missing here. Samuel
[Qemu-devel] [PATCH] linux-user: Fix sched_get/setaffinity conversion
sched_get/setaffinity linux-user syscalls were missing conversions for
little/big endian, which is hairy since longs may not be the same size
either.
For simplicity, this just introduces loops to convert bit by bit like is
done for select.
Signed-off-by: Samuel Thibault
---
linux-user/syscall.c | 75 ++--
1 file changed, 67 insertions(+), 8 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 11c9116c4a..f806cfb9a7 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7716,6 +7716,67 @@ static TargetFdTrans target_inotify_trans = {
};
#endif
+static int target_to_host_cpu_mask(unsigned long *host_mask, size_t host_size,
abi_ulong target_addr, size_t target_size)
+{
+unsigned target_bits = sizeof(abi_ulong) * 8;
+unsigned host_bits = sizeof(*host_mask) * 8;
+abi_ulong *target_mask;
+unsigned i, j;
+
+assert(host_size >= target_size);
+
+target_mask = lock_user(VERIFY_READ, target_addr, target_size, 1);
+if (!target_mask) {
+return -TARGET_EFAULT;
+}
+memset(host_mask, 0, host_size);
+
+for (i = 0 ; i < target_size / sizeof(abi_ulong); i++) {
+unsigned bit = i * target_bits;
+abi_ulong val;
+
+__get_user(val, &target_mask[i]);
+for (j = 0; j < target_bits; j++, bit++) {
+if (val & (1UL << j)) {
+host_mask[bit / host_bits] |= 1UL << (bit % host_bits);
+}
+}
+}
+
+unlock_user(target_mask, target_addr, 0);
+return 0;
+}
+
+static int host_to_target_cpu_mask(const unsigned long *host_mask, size_t
host_size, abi_ulong target_addr, size_t target_size)
+{
+unsigned target_bits = sizeof(abi_ulong) * 8;
+unsigned host_bits = sizeof(*host_mask) * 8;
+abi_ulong *target_mask;
+unsigned i, j;
+
+assert(host_size >= target_size);
+
+target_mask = lock_user(VERIFY_WRITE, target_addr, target_size, 0);
+if (!target_mask) {
+return -TARGET_EFAULT;
+}
+
+for (i = 0 ; i < target_size / sizeof(abi_ulong); i++) {
+unsigned bit = i * target_bits;
+abi_ulong val = 0;
+
+for (j = 0; j < target_bits; j++, bit++) {
+if (host_mask[bit / host_bits] & (1UL << (bit % host_bits))) {
+val |= 1UL << j;
+}
+}
+__put_user(val, &target_mask[i]);
+}
+
+unlock_user(target_mask, target_addr, target_size);
+return 0;
+}
+
/* do_syscall() should always have a single exit point at the end so
that actions, such as logging of syscall results, can be performed.
All errnos that do_syscall() returns must be -TARGET_. */
@@ -10353,6 +10414,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
mask_size = (arg2 + (sizeof(*mask) - 1)) & ~(sizeof(*mask) - 1);
mask = alloca(mask_size);
+memset(mask, 0, mask_size);
ret = get_errno(sys_sched_getaffinity(arg1, mask_size, mask));
if (!is_error(ret)) {
@@ -10372,9 +10434,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
ret = arg2;
}
-if (copy_to_user(arg3, mask, ret)) {
-goto efault;
-}
+ret = host_to_target_cpu_mask(mask, mask_size, arg3, arg2);
}
}
break;
@@ -10392,13 +10452,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
break;
}
mask_size = (arg2 + (sizeof(*mask) - 1)) & ~(sizeof(*mask) - 1);
-
mask = alloca(mask_size);
-if (!lock_user_struct(VERIFY_READ, p, arg3, 1)) {
-goto efault;
+
+ret = target_to_host_cpu_mask(mask, mask_size, arg3, arg2);
+if (ret) {
+break;
}
-memcpy(mask, p, arg2);
-unlock_user_struct(p, arg2, 0);
ret = get_errno(sys_sched_setaffinity(arg1, mask_size, mask));
}
--
2.15.1
Re: [Qemu-devel] [PATCH] linux-user: Fix sched_get/setaffinity conversion
Laurent Vivier, on jeu. 28 déc. 2017 18:50:14 +0100, wrote:
> > +for (j = 0; j < abi_ubits; j++, bit++) {
> > +if (mask[bit / ubits] & (1UL << (bit % ubits))) {
>
> You should use __CPUMASK() and introduce a TARGET_CPUMASK().
Err, do we really want to use such insides of glibc?
Samuel
[Qemu-devel] [PATCH] linux-user: Add getcpu() support
Signed-off-by: Samuel Thibault
---
linux-user/syscall.c | 16
1 file changed, 16 insertions(+)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 8ec7de96ce..bb8cb726f5 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -296,6 +296,8 @@ _syscall3(int, sys_sched_getaffinity, pid_t, pid, unsigned
int, len,
#define __NR_sys_sched_setaffinity __NR_sched_setaffinity
_syscall3(int, sys_sched_setaffinity, pid_t, pid, unsigned int, len,
unsigned long *, user_mask_ptr);
+#define __NR_sys_getcpu __NR_getcpu
+_syscall3(int, sys_getcpu, unsigned *, cpu, unsigned *, node, void *, tcache);
_syscall4(int, reboot, int, magic1, int, magic2, unsigned int, cmd,
void *, arg);
_syscall2(int, capget, struct __user_cap_header_struct *, header,
@@ -10443,6 +10445,20 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
ret = get_errno(sys_sched_setaffinity(arg1, mask_size, mask));
}
break;
+case TARGET_NR_getcpu:
+{
+unsigned cpu, node;
+ret = get_errno(sys_getcpu(arg1 ? &cpu : NULL,
+ arg2 ? &node : NULL,
+ NULL));
+if (arg1) {
+put_user(cpu, arg1, abi_uint);
+}
+if (arg2) {
+put_user(node, arg2, abi_uint);
+}
+}
+break;
case TARGET_NR_sched_setparam:
{
struct sched_param *target_schp;
--
2.15.1
[Qemu-devel] [PATCH] linux-user: Fix sched_get/setaffinity conversion
sched_get/setaffinity linux-user syscalls were missing conversions for
little/big endian, which is hairy since longs may not be the same size
either.
For simplicity, this just introduces loops to convert bit by bit like is
done for select.
Signed-off-by: Samuel Thibault
---
linux-user/syscall.c | 48
1 file changed, 44 insertions(+), 4 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 11c9116c4a..8ec7de96ce 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10341,6 +10341,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
{
unsigned int mask_size;
unsigned long *mask;
+abi_ulong *abimask;
+unsigned i, j;
/*
* sched_getaffinity needs multiples of ulong, so need to take
@@ -10353,6 +10355,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
mask_size = (arg2 + (sizeof(*mask) - 1)) & ~(sizeof(*mask) - 1);
mask = alloca(mask_size);
+memset(mask, 0, mask_size);
ret = get_errno(sys_sched_getaffinity(arg1, mask_size, mask));
if (!is_error(ret)) {
@@ -10372,9 +10375,27 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
ret = arg2;
}
-if (copy_to_user(arg3, mask, ret)) {
+abimask = lock_user(VERIFY_WRITE, arg3, arg2, 0);
+if (!abimask) {
goto efault;
}
+
+for (i = 0 ; i < arg2 / sizeof(abi_ulong); i++) {
+unsigned abi_ubits = sizeof(abi_ulong) * 8;
+unsigned ubits = sizeof(*mask) * 8;
+unsigned bit = i * abi_ubits;
+abi_ulong val = 0;
+
+for (j = 0; j < abi_ubits; j++, bit++) {
+if (mask[bit / ubits] & (1UL << (bit % ubits))) {
+val |= 1UL << j;
+}
+}
+__put_user(val, &abimask[i]);
+}
+
+unlock_user(abimask, arg3, arg2);
+
}
}
break;
@@ -10382,6 +10403,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
{
unsigned int mask_size;
unsigned long *mask;
+abi_ulong *abimask;
+unsigned i, j;
/*
* sched_setaffinity needs multiples of ulong, so need to take
@@ -10394,11 +10417,28 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
mask_size = (arg2 + (sizeof(*mask) - 1)) & ~(sizeof(*mask) - 1);
mask = alloca(mask_size);
-if (!lock_user_struct(VERIFY_READ, p, arg3, 1)) {
+memset(mask, 0, mask_size);
+
+abimask = lock_user(VERIFY_READ, arg3, arg2, 1);
+if (!abimask) {
goto efault;
}
-memcpy(mask, p, arg2);
-unlock_user_struct(p, arg2, 0);
+
+for (i = 0 ; i < arg2 / sizeof(abi_ulong); i++) {
+unsigned abi_ubits = sizeof(abi_ulong) * 8;
+unsigned ubits = sizeof(*mask) * 8;
+unsigned bit = i * abi_ubits;
+abi_ulong val;
+
+__get_user(val, &abimask[i]);
+for (j = 0; j < abi_ubits; j++, bit++) {
+if (val & (1UL << j)) {
+mask[bit / ubits] |= 1UL << (bit % ubits);
+}
+}
+}
+
+unlock_user(abimask, arg3, 0);
ret = get_errno(sys_sched_setaffinity(arg1, mask_size, mask));
}
--
2.15.1
Re: [Qemu-devel] [PATCH for-2.12 2/2] net: Remove the deprecated -tftp, -bootp, -redir and -smb options
Thomas Huth, on jeu. 07 déc. 2017 19:02:35 +0100, wrote:
> These options likely do not work as expected as soon as the user
> tries to use more than one network interface at once. The parameters
> have been marked as deprecated since QEMU v2.6, so users had plenty
> of time to move their scripts to the new syntax. Time to remove the
> old parameters now.
>
> Signed-off-by: Thomas Huth
Reviewed-by: Samuel Thibault
> ---
> include/net/net.h | 3 ---
> include/net/slirp.h | 4
> net/slirp.c | 58
> -
> os-posix.c | 8
> qemu-doc.texi | 24 --
> qemu-options.hx | 15 --
> vl.c| 18 -
> 7 files changed, 130 deletions(-)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 1c55a93..670e03e 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -204,9 +204,6 @@ extern NICInfo nd_table[MAX_NICS];
> extern const char *host_net_devices[];
>
> /* from net.c */
> -extern const char *legacy_tftp_prefix;
> -extern const char *legacy_bootp_filename;
> -
> int net_client_init(QemuOpts *opts, bool is_netdev, Error **errp);
> int net_client_parse(QemuOptsList *opts_list, const char *str);
> int net_init_clients(void);
> diff --git a/include/net/slirp.h b/include/net/slirp.h
> index 0c98e46..2c37fa0 100644
> --- a/include/net/slirp.h
> +++ b/include/net/slirp.h
> @@ -34,10 +34,6 @@
> void hmp_hostfwd_add(Monitor *mon, const QDict *qdict);
> void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict);
>
> -int net_slirp_redir(const char *redir_str);
> -
> -int net_slirp_smb(const char *exported_dir);
> -
> void hmp_info_usernet(Monitor *mon, const QDict *qdict);
>
> #endif
> diff --git a/net/slirp.c b/net/slirp.c
> index cb8ca23..4999a25 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -85,8 +85,6 @@ typedef struct SlirpState {
> } SlirpState;
>
> static struct slirp_config_str *slirp_configs;
> -const char *legacy_tftp_prefix;
> -const char *legacy_bootp_filename;
> static QTAILQ_HEAD(slirp_stacks, SlirpState) slirp_stacks =
> QTAILQ_HEAD_INITIALIZER(slirp_stacks);
>
> @@ -96,8 +94,6 @@ static int slirp_guestfwd(SlirpState *s, const char
> *config_str,
>int legacy_format, Error **errp);
>
> #ifndef _WIN32
> -static const char *legacy_smb_export;
> -
> static int slirp_smb(SlirpState *s, const char *exported_dir,
> struct in_addr vserver_addr, Error **errp);
> static void slirp_smb_cleanup(SlirpState *s);
> @@ -193,13 +189,6 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
> return -1;
> }
>
> -if (!tftp_export) {
> -tftp_export = legacy_tftp_prefix;
> -}
> -if (!bootfile) {
> -bootfile = legacy_bootp_filename;
> -}
> -
> if (vnetwork) {
> if (get_str_sep(buf, sizeof(buf), &vnetwork, '/') < 0) {
> if (!inet_aton(vnetwork, &net)) {
> @@ -386,9 +375,6 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
> }
> }
> #ifndef _WIN32
> -if (!smb_export) {
> -smb_export = legacy_smb_export;
> -}
> if (smb_export) {
> if (slirp_smb(s, smb_export, smbsrv, errp) < 0) {
> goto error;
> @@ -586,28 +572,6 @@ void hmp_hostfwd_add(Monitor *mon, const QDict *qdict)
>
> }
>
> -int net_slirp_redir(const char *redir_str)
> -{
> -struct slirp_config_str *config;
> -Error *err = NULL;
> -int res;
> -
> -if (QTAILQ_EMPTY(&slirp_stacks)) {
> -config = g_malloc(sizeof(*config));
> -pstrcpy(config->str, sizeof(config->str), redir_str);
> -config->flags = SLIRP_CFG_HOSTFWD | SLIRP_CFG_LEGACY;
> -config->next = slirp_configs;
> -slirp_configs = config;
> -return 0;
> -}
> -
> -res = slirp_hostfwd(QTAILQ_FIRST(&slirp_stacks), redir_str, 1, &err);
> -if (res < 0) {
> -error_report_err(err);
> -}
> -return res;
> -}
> -
> #ifndef _WIN32
>
> /* automatic user mode samba server configuration */
> @@ -723,28 +687,6 @@ static int slirp_smb(SlirpState* s, const char
> *exported_dir,
> return 0;
> }
>
> -/* automatic user mode samba server configuration (legacy interface) */
> -int net_slirp_smb(const char *exported_dir)
> -{
> -struct in_addr vserver_addr = { .s_addr = 0 };
> -
> -if (legacy_smb_export) {
> -fprintf(stderr, "-smb gi
Re: [Qemu-devel] [PATCH for-2.12 1/2] net: Remove the legacy "-net channel" parameter
Thomas Huth, on jeu. 07 déc. 2017 19:02:34 +0100, wrote:
> It has never been documented, so hardly anybody knows about this
> parameter, and it is marked as deprecated since QEMU v2.6.
> Time to let it go now.
>
> Signed-off-by: Thomas Huth
Reviewed-by: Samuel Thibault
> ---
> include/net/slirp.h | 2 --
> net/net.c | 7 ---
> net/slirp.c | 34 --
> qemu-doc.texi | 5 -
> 4 files changed, 48 deletions(-)
>
> diff --git a/include/net/slirp.h b/include/net/slirp.h
> index 64b795c..0c98e46 100644
> --- a/include/net/slirp.h
> +++ b/include/net/slirp.h
> @@ -36,8 +36,6 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict);
>
> int net_slirp_redir(const char *redir_str);
>
> -int net_slirp_parse_legacy(QemuOptsList *opts_list, const char *optarg, int
> *ret);
> -
> int net_slirp_smb(const char *exported_dir);
>
> void hmp_info_usernet(Monitor *mon, const QDict *qdict);
> diff --git a/net/net.c b/net/net.c
> index 39ef546..7425857 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1565,13 +1565,6 @@ int net_init_clients(void)
>
> int net_client_parse(QemuOptsList *opts_list, const char *optarg)
> {
> -#if defined(CONFIG_SLIRP)
> -int ret;
> -if (net_slirp_parse_legacy(opts_list, optarg, &ret)) {
> -return ret;
> -}
> -#endif
> -
> if (!qemu_opts_parse_noisily(opts_list, optarg, true)) {
> return -1;
> }
> diff --git a/net/slirp.c b/net/slirp.c
> index 318a26e..cb8ca23 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -956,37 +956,3 @@ int net_init_slirp(const Netdev *netdev, const char
> *name,
>
> return ret;
> }
> -
> -int net_slirp_parse_legacy(QemuOptsList *opts_list, const char *optarg, int
> *ret)
> -{
> -if (strcmp(opts_list->name, "net") != 0 ||
> -strncmp(optarg, "channel,", strlen("channel,")) != 0) {
> -return 0;
> -}
> -
> -error_report("The '-net channel' option is deprecated. "
> - "Please use '-netdev user,guestfwd=...' instead.");
> -
> -/* handle legacy -net channel,port:chr */
> -optarg += strlen("channel,");
> -
> -if (QTAILQ_EMPTY(&slirp_stacks)) {
> -struct slirp_config_str *config;
> -
> -config = g_malloc(sizeof(*config));
> -pstrcpy(config->str, sizeof(config->str), optarg);
> -config->flags = SLIRP_CFG_LEGACY;
> -config->next = slirp_configs;
> -slirp_configs = config;
> -*ret = 0;
> -} else {
> -Error *err = NULL;
> -*ret = slirp_guestfwd(QTAILQ_FIRST(&slirp_stacks), optarg, 1, &err);
> -if (*ret < 0) {
> -error_report_err(err);
> -}
> -}
> -
> -return 1;
> -}
> -
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index db2351c..982cab5 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -2459,11 +2459,6 @@ The ``-smb /some/dir'' argument is now a synonym for
> setting
> the ``-netdev user,smb=/some/dir'' argument instead. The new
> syntax allows different settings to be provided per NIC.
>
> -@subsection -net channel (since 2.6.0)
> -
> -The ``--net channel,ARGS'' argument is now a synonym for setting
> -the ``-netdev user,guestfwd=ARGS'' argument instead.
> -
> @subsection -net vlan (since 2.9.0)
>
> The ``-net vlan=NN'' argument is partially replaced with the
> --
> 1.8.3.1
>
--
Samuel
R: Parce que ça renverse bêtement l'ordre naturel de lecture!
Q: Mais pourquoi citer en fin d'article est-il si effroyable?
R: Citer en fin d'article
Q: Quelle est la chose la plus désagréable sur les groupes de news?
Re: [Qemu-devel] [PATCH] baum: Truncate braille device size to 84x1
Eric Blake, on lun. 11 déc. 2017 08:30:39 -0600, wrote:
> On 12/10/2017 06:19 PM, Samuel Thibault wrote:
> > Baum device bigger than 84 do not actually exist, some guest drivers
> > would be upset by such sizes.
> >
> > Signed-off-by: Samuel Thibault
> > ---
> > chardev/baum.c | 8 +++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
>
> > @@ -239,6 +239,12 @@ static int baum_deferred_init(BaumChardev *baum)
> > brlapi_perror("baum: brlapi__getDisplaySize");
> > return 0;
> > }
> > +if (baum->y > 1) {
> > +baum->y = 1;
> > +}
> > +if (baum->x > 84) {
> > +baum->x = 84;
> > +}
>
> Is magic clamping desirable, or is it better to make it a hard error if
> the user configured a size that is not possible?
The thing is: the user didn't configure something, she just happened to
use a braille device bigger than 84 to display qemu's braille output.
This is the same situation as for the virtual video card: qemu could
expose resolutions as big as the size of the X display where qemu is
running on, but it's not a good idea to expose them all because some
drivers could go crazy with sizes bigger than what is supposed to be
supported by the hardware (or just assume the device is bogus and refuse
to drive it), and one should thus rather clamp them to what an actual
video device would support.
Samuel
[Qemu-devel] [PATCH] baum: Truncate braille device size to 84x1
Baum device bigger than 84 do not actually exist, some guest drivers
would be upset by such sizes.
Signed-off-by: Samuel Thibault
---
chardev/baum.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/chardev/baum.c b/chardev/baum.c
index 67fd783a59..78b0c87625 100644
--- a/chardev/baum.c
+++ b/chardev/baum.c
@@ -1,7 +1,7 @@
/*
* QEMU Baum Braille Device
*
- * Copyright (c) 2008, 2010-2011, 2016 Samuel Thibault
+ * Copyright (c) 2008, 2010-2011, 2016-2017 Samuel Thibault
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
deal
@@ -239,6 +239,12 @@ static int baum_deferred_init(BaumChardev *baum)
brlapi_perror("baum: brlapi__getDisplaySize");
return 0;
}
+if (baum->y > 1) {
+baum->y = 1;
+}
+if (baum->x > 84) {
+baum->x = 84;
+}
con = qemu_console_lookup_by_index(0);
if (con && qemu_console_is_graphic(con)) {
--
2.15.0
Re: [Qemu-devel] [PULL 0/1] baum braille device update
Peter Maydell, on lun. 04 déc. 2017 15:36:32 +, wrote: > On 4 December 2017 at 15:26, Samuel Thibault wrote: > > Peter Maydell, on lun. 04 déc. 2017 15:21:48 +, wrote: > >> On 4 December 2017 at 15:17, Samuel Thibault > >> wrote: > >> > ---- > >> > Samuel Thibault (1): > >> > baum: Truncate braille device size to 84x1 > >> > >> Hi. Is this pull request intended for 2.11? > > > > If possible, yes. > > > >> We're currently in freeze for that and only taking "absolutely release > >> critical" bug fixes... > > > > Ok, as you wish. > > > > It's a relatively corner case: a friend of mine does have a 88-cell > > braille display, and can not use it with guests by running qemu > > -usbdevice braille due to this bug. > > Mmm. Looking at the code for baum.c for 2.10, this isn't a > regression from 2.10 or a security issue, which is what it > would need to be for us to want to put this in at this point. Ah, ok, nevermind then, it'll have to wait. Sorry I didn't know that. Samuel
Re: [Qemu-devel] [PULL 0/1] baum braille device update
Hello, Peter Maydell, on lun. 04 déc. 2017 15:21:48 +, wrote: > On 4 December 2017 at 15:17, Samuel Thibault > wrote: > > warning: redirection vers https://people.debian.org/~sthibault/qemu.git/ > > The following changes since commit 7edaf99759017d3e175e37cffc3536e86a3bd380: > > > > Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into > > staging (2017-11-13 13:54:59 +) > > > > are available in the Git repository at: > > > > http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault > > > > for you to fetch changes up to 15405525fce773d43e7e5cabc2b9ac4b9d4d7672: > > > > baum: Truncate braille device size to 84x1 (2017-12-03 19:35:50 +0100) > > > > ---- > > baum updates > > > > > > Samuel Thibault (1): > > baum: Truncate braille device size to 84x1 > > Hi. Is this pull request intended for 2.11? If possible, yes. > We're currently in freeze for that and only taking "absolutely release > critical" bug fixes... Ok, as you wish. It's a relatively corner case: a friend of mine does have a 88-cell braille display, and can not use it with guests by running qemu -usbdevice braille due to this bug. Samuel
[Qemu-devel] [PULL 1/1] baum: Truncate braille device size to 84x1
Baum device bigger than 84 do not actually exist, some guest drivers
would be upset by such sizes.
Signed-off-by: Samuel Thibault
---
chardev/baum.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/chardev/baum.c b/chardev/baum.c
index 67fd783a59..78b0c87625 100644
--- a/chardev/baum.c
+++ b/chardev/baum.c
@@ -1,7 +1,7 @@
/*
* QEMU Baum Braille Device
*
- * Copyright (c) 2008, 2010-2011, 2016 Samuel Thibault
+ * Copyright (c) 2008, 2010-2011, 2016-2017 Samuel Thibault
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
deal
@@ -239,6 +239,12 @@ static int baum_deferred_init(BaumChardev *baum)
brlapi_perror("baum: brlapi__getDisplaySize");
return 0;
}
+if (baum->y > 1) {
+baum->y = 1;
+}
+if (baum->x > 84) {
+baum->x = 84;
+}
con = qemu_console_lookup_by_index(0);
if (con && qemu_console_is_graphic(con)) {
--
2.15.0
[Qemu-devel] [PULL 0/1] baum braille device update
warning: redirection vers https://people.debian.org/~sthibault/qemu.git/ The following changes since commit 7edaf99759017d3e175e37cffc3536e86a3bd380: Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into staging (2017-11-13 13:54:59 +) are available in the Git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 15405525fce773d43e7e5cabc2b9ac4b9d4d7672: baum: Truncate braille device size to 84x1 (2017-12-03 19:35:50 +0100) baum updates -------- Samuel Thibault (1): baum: Truncate braille device size to 84x1 chardev/baum.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-)
Re: [Qemu-devel] [PATCH] slirp: don't zero ti_i since we access it later.
Tao Wu(吴涛@Eng), on jeu. 09 nov. 2017 10:48:27 -0800, wrote: > Thanks. Actually this is a follow up with my previous effort to fix this bug. > I was busy on something else and then got lost in that old thread. Now I just > checked some my local patch > to see if they've merged to upstream and then found it out. > > This is old thread about this: > [1]http://lists.nongnu.org/archive/html/qemu-devel/2017-04/msg05544.html I knew I had seen that proposal somewhere before :) Thanks for the patch, Samuel
[Qemu-devel] [PULL 0/1] slirp update
warning: redirection vers https://people.debian.org/~sthibault/qemu.git/ The following changes since commit b0fbe46ad82982b289a44ee2495b59b0bad8a842: Update version for v2.11.0-rc0 release (2017-11-07 16:05:28 +) are available in the git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 990132cda9baa27bdc558df6c9c15aacb0322d2c: slirp: don't zero the whole ti_i when m == NULL (2017-11-09 18:59:22 +0100) slirp updates Tao Wu (1): slirp: don't zero the whole ti_i when m == NULL This fixes the qemu slirp behavior is some rare cases (some RST cases, keep alive probes), packets would be sent to 0.0.0.0. slirp/tcp_subr.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-)
[Qemu-devel] [PULL 1/1] slirp: don't zero the whole ti_i when m == NULL
From: Tao Wu
98c63057d2144fb81681580cd84c13c93794c96e ('slirp: Factorizing
tcpiphdr structure with an union') introduced a memset call to clear
possibly-undefined fields in ti. This however overwrites src/dst/pr which
are used below.
So let us clear only the unused fields.
This should fix some rare cases (some RST cases, keep alive probes)
where packets would be sent to 0.0.0.0.
Signed-off-by: Tao Wu
Signed-off-by: Samuel Thibault
---
slirp/tcp_subr.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index dc8b4bbb50..da0d53743f 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -148,7 +148,16 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti, struct
mbuf *m,
m->m_data += IF_MAXLINKHDR;
*mtod(m, struct tcpiphdr *) = *ti;
ti = mtod(m, struct tcpiphdr *);
- memset(&ti->ti, 0, sizeof(ti->ti));
+ switch (af) {
+ case AF_INET:
+ ti->ti.ti_i4.ih_x1 = 0;
+ break;
+ case AF_INET6:
+ ti->ti.ti_i6.ih_x1 = 0;
+ break;
+ default:
+ g_assert_not_reached();
+ }
flags = TH_ACK;
} else {
/*
--
2.14.2
Re: [Qemu-devel] [PATCH] slirp: don't zero ti_i since we access it later.
Hello, Tao Wu, on mer. 08 nov. 2017 14:53:40 -0800, wrote: > The current code looks buggy, we zero ti_i while we access > ti_dst/ti_src later. Mmm, indeed, looking again at how it was introduced, it was too much. Samuel
Re: [Qemu-devel] [PATCH 36/88] SLIRP: use g_new() family of functions
Hello,
Philippe Mathieu-Daudé, on ven. 06 oct. 2017 20:49:31 -0300, wrote:
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -869,7 +869,7 @@ net_init_slirp_configs(const StringList *fwd, int flags)
> while (fwd) {
> struct slirp_config_str *config;
>
> -config = g_malloc0(sizeof(*config));
> +config = g_new0(struct slirp_config_str, 1);
It doesn't really seem like an improvement to me in such case: we don't
have overflow issues here, and using sizeof(*config) makes the line
clearly correct, while expliciting the struct behind means we need to
make sure of the coherency.
> diff --git a/slirp/socket.c b/slirp/socket.c
> index cb7b5b608d..2eccb68c2e 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -48,7 +48,7 @@ socreate(Slirp *slirp)
> {
>struct socket *so;
>
> - so = (struct socket *)malloc(sizeof(struct socket));
> + so = g_new(struct socket, 1);
Similarly, here I'd rather write g_malloc(sizeof(*so));
Samuel
[Qemu-devel] [PULL 3/3] slirp: Add a special case for the NULL socket
From: Kevin Cernekee
NULL sockets are used for NDP, BOOTP, and other critical operations.
If the topmost mbuf in a NULL session is blocked pending resolution,
it may cause problems if it blocks other packets with a NULL socket.
So do not add mbufs with a NULL socket field to the same session.
Signed-off-by: Kevin Cernekee
Signed-off-by: Samuel Thibault
---
slirp/if.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/slirp/if.c b/slirp/if.c
index 6262d77495..590753c658 100644
--- a/slirp/if.c
+++ b/slirp/if.c
@@ -73,14 +73,16 @@ if_output(struct socket *so, struct mbuf *ifm)
* We mustn't put this packet back on the fastq (or we'll send it out
of order)
* XXX add cache here?
*/
- for (ifq = (struct mbuf *) slirp->if_batchq.qh_rlink;
-(struct quehead *) ifq != &slirp->if_batchq;
-ifq = ifq->ifq_prev) {
- if (so == ifq->ifq_so) {
- /* A match! */
- ifm->ifq_so = so;
- ifs_insque(ifm, ifq->ifs_prev);
- goto diddit;
+ if (so) {
+ for (ifq = (struct mbuf *) slirp->if_batchq.qh_rlink;
+(struct quehead *) ifq != &slirp->if_batchq;
+ifq = ifq->ifq_prev) {
+ if (so == ifq->ifq_so) {
+ /* A match! */
+ ifm->ifq_so = so;
+ ifs_insque(ifm, ifq->ifs_prev);
+ goto diddit;
+ }
}
}
--
2.14.1
[Qemu-devel] [PULL 1/3] slirp: Add explanation for hostfwd parsing failure
From: "Dr. David Alan Gilbert"
e.g.
./x86_64-softmmu/qemu-system-x86_64 -nographic -netdev
'user,id=vnet,hostfwd=:555.0.0.0:0-:22'
qemu-system-x86_64: -netdev user,id=vnet,hostfwd=:555.0.0.0:0-:22: Invalid host
forwarding rule ':555.0.0.0:0-:22' (Bad host address)
Signed-off-by: Dr. David Alan Gilbert
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Samuel Thibault
---
net/slirp.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/net/slirp.c b/net/slirp.c
index 01ed21c006..318a26e892 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -496,9 +496,11 @@ static int slirp_hostfwd(SlirpState *s, const char
*redir_str,
char buf[256];
int is_udp;
char *end;
+const char *fail_reason = "Unknown reason";
p = redir_str;
if (!p || get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
+fail_reason = "No : separators";
goto fail_syntax;
}
if (!strcmp(buf, "tcp") || buf[0] == '\0') {
@@ -506,35 +508,43 @@ static int slirp_hostfwd(SlirpState *s, const char
*redir_str,
} else if (!strcmp(buf, "udp")) {
is_udp = 1;
} else {
+fail_reason = "Bad protocol name";
goto fail_syntax;
}
if (!legacy_format) {
if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
+fail_reason = "Missing : separator";
goto fail_syntax;
}
if (buf[0] != '\0' && !inet_aton(buf, &host_addr)) {
+fail_reason = "Bad host address";
goto fail_syntax;
}
}
if (get_str_sep(buf, sizeof(buf), &p, legacy_format ? ':' : '-') < 0) {
+fail_reason = "Bad host port separator";
goto fail_syntax;
}
host_port = strtol(buf, &end, 0);
if (*end != '\0' || host_port < 0 || host_port > 65535) {
+fail_reason = "Bad host port";
goto fail_syntax;
}
if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
+fail_reason = "Missing guest address";
goto fail_syntax;
}
if (buf[0] != '\0' && !inet_aton(buf, &guest_addr)) {
+fail_reason = "Bad guest address";
goto fail_syntax;
}
guest_port = strtol(p, &end, 0);
if (*end != '\0' || guest_port < 1 || guest_port > 65535) {
+fail_reason = "Bad guest port";
goto fail_syntax;
}
@@ -547,7 +557,8 @@ static int slirp_hostfwd(SlirpState *s, const char
*redir_str,
return 0;
fail_syntax:
-error_setg(errp, "Invalid host forwarding rule '%s'", redir_str);
+error_setg(errp, "Invalid host forwarding rule '%s' (%s)", redir_str,
+ fail_reason);
return -1;
}
--
2.14.1
[Qemu-devel] [PULL 2/3] slirp: Fix intermittent send queue hangs on a socket
From: Kevin Cernekee
if_output() originally sent one mbuf per call and used the slirp->next_m
variable to keep track of where it left off. But nowadays it tries to
send all of the mbufs from the fastq, and one mbuf from each session on
the batchq. The next_m variable is both redundant and harmful: there is
a case[0] involving delayed packets in which next_m ends up pointing
to &slirp->if_batchq when an active session still exists, and this
blocks all traffic for that session until qemu is restarted.
The test case was created to reproduce a problem that was seen on
long-running Chromium OS VM tests[1] which rapidly create and
destroy ssh connections through hostfwd.
[0] https://pastebin.com/NNy6LreF
[1] https://bugs.chromium.org/p/chromium/issues/detail?id=766323
Signed-off-by: Kevin Cernekee
Signed-off-by: Samuel Thibault
---
slirp/if.c| 51 +--
slirp/slirp.h | 1 -
2 files changed, 17 insertions(+), 35 deletions(-)
diff --git a/slirp/if.c b/slirp/if.c
index 51ae0d0e9a..6262d77495 100644
--- a/slirp/if.c
+++ b/slirp/if.c
@@ -30,7 +30,6 @@ if_init(Slirp *slirp)
{
slirp->if_fastq.qh_link = slirp->if_fastq.qh_rlink = &slirp->if_fastq;
slirp->if_batchq.qh_link = slirp->if_batchq.qh_rlink = &slirp->if_batchq;
-slirp->next_m = (struct mbuf *) &slirp->if_batchq;
}
/*
@@ -100,10 +99,6 @@ if_output(struct socket *so, struct mbuf *ifm)
}
} else {
ifq = (struct mbuf *) slirp->if_batchq.qh_rlink;
-/* Set next_m if the queue was empty so far */
-if ((struct quehead *) slirp->next_m == &slirp->if_batchq) {
-slirp->next_m = ifm;
-}
}
/* Create a new doubly linked list for this session */
@@ -143,21 +138,18 @@ diddit:
}
/*
- * Send a packet
- * We choose a packet based on its position in the output queues;
+ * Send one packet from each session.
* If there are packets on the fastq, they are sent FIFO, before
- * everything else. Otherwise we choose the first packet from the
- * batchq and send it. the next packet chosen will be from the session
- * after this one, then the session after that one, and so on.. So,
- * for example, if there are 3 ftp session's fighting for bandwidth,
+ * everything else. Then we choose the first packet from each
+ * batchq session (socket) and send it.
+ * For example, if there are 3 ftp sessions fighting for bandwidth,
* one packet will be sent from the first session, then one packet
- * from the second session, then one packet from the third, then back
- * to the first, etc. etc.
+ * from the second session, then one packet from the third.
*/
void if_start(Slirp *slirp)
{
uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
-bool from_batchq, next_from_batchq;
+bool from_batchq = false;
struct mbuf *ifm, *ifm_next, *ifqt;
DEBUG_CALL("if_start");
@@ -167,26 +159,29 @@ void if_start(Slirp *slirp)
}
slirp->if_start_busy = true;
+struct mbuf *batch_head = NULL;
+if (slirp->if_batchq.qh_link != &slirp->if_batchq) {
+batch_head = (struct mbuf *) slirp->if_batchq.qh_link;
+}
+
if (slirp->if_fastq.qh_link != &slirp->if_fastq) {
ifm_next = (struct mbuf *) slirp->if_fastq.qh_link;
-next_from_batchq = false;
-} else if ((struct quehead *) slirp->next_m != &slirp->if_batchq) {
-/* Nothing on fastq, pick up from batchq via next_m */
-ifm_next = slirp->next_m;
-next_from_batchq = true;
+} else if (batch_head) {
+/* Nothing on fastq, pick up from batchq */
+ifm_next = batch_head;
+from_batchq = true;
} else {
ifm_next = NULL;
}
while (ifm_next) {
ifm = ifm_next;
-from_batchq = next_from_batchq;
ifm_next = ifm->ifq_next;
if ((struct quehead *) ifm_next == &slirp->if_fastq) {
/* No more packets in fastq, switch to batchq */
-ifm_next = slirp->next_m;
-next_from_batchq = true;
+ifm_next = batch_head;
+from_batchq = true;
}
if ((struct quehead *) ifm_next == &slirp->if_batchq) {
/* end of batchq */
@@ -199,11 +194,6 @@ void if_start(Slirp *slirp)
continue;
}
-if (ifm == slirp->next_m) {
-/* Set which packet to send on next iteration */
-slirp->next_m = ifm->ifq_next;
-}
-
/* Remove it from the queue */
ifqt = ifm->ifq_prev;
remque(ifm);
@@ -214,15 +204,8 @@ void if_start(Slirp *slirp)
insque(next, ifqt);
ifs_remque(ifm);
-
if (!from_batchq) {
-/* Next packet in fastq is from the same session */
[Qemu-devel] [PULL 0/3] slirp updates
warning: redirection vers https://people.debian.org/~sthibault/qemu.git/ The following changes since commit 460b6c8e581aa06b86f59eebd9e52edfe7adf417: Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2017-09-23 12:55:40 +0100) are available in the git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 13146a83951e045c810c37c5c11c2a016ebc0663: slirp: Add a special case for the NULL socket (2017-09-24 20:04:09 +0200) slirp updates Dr. David Alan Gilbert (1): slirp: Add explanation for hostfwd parsing failure Kevin Cernekee (2): slirp: Fix intermittent send queue hangs on a socket slirp: Add a special case for the NULL socket net/slirp.c | 13 ++- slirp/if.c| 69 +++ slirp/slirp.h | 1 - 3 files changed, 39 insertions(+), 44 deletions(-)
Re: [Qemu-devel] [PATCH 2/2] slirp: Add a special case for the NULL socket
Kevin Cernekee, on mer. 20 sept. 2017 13:42:05 -0700, wrote: > NULL sockets are used for NDP, BOOTP, and other critical operations. > If the topmost mbuf in a NULL session is blocked pending resolution, > it may cause problems if it blocks other packets with a NULL socket. > So do not add mbufs with a NULL socket field to the same session. That makes a lot of sense indeed, applied to my tree. Thanks! Samuel
Re: [Qemu-devel] [PATCH 1/2] slirp: Fix intermittent send queue hangs on a socket
Hello, Kevin Cernekee, on mer. 20 sept. 2017 13:42:04 -0700, wrote: > if_output() originally sent one mbuf per call and used the slirp->next_m > variable to keep track of where it left off. But nowadays it tries to > send all of the mbufs from the fastq, and one mbuf from each session on > the batchq. The next_m variable is both redundant and harmful: there is > a case[0] involving delayed packets in which next_m ends up pointing > to &slirp->if_batchq when an active session still exists, and this > blocks all traffic for that session until qemu is restarted. That also makes things simpler, I applied it to my tree, thanks! Samuel
Re: [Qemu-devel] [PATCH] slirp: Add explanation for hostfwd parsing failure
Philippe Mathieu-Daudé, on ven. 08 sept. 2017 13:19:56 -0300, wrote: > Hi David, > > On 09/08/2017 12:53 PM, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" > > > > e.g. > > ./x86_64-softmmu/qemu-system-x86_64 -nographic -netdev > > 'user,id=vnet,hostfwd=:555.0.0.0:0-:22' > > qemu-system-x86_64: -netdev user,id=vnet,hostfwd=:555.0.0.0:0-:22: Invalid > > host forwarding rule ':555.0.0.0:0-:22' (Bad host address) > > > > Signed-off-by: Dr. David Alan Gilbert > > --- > > net/slirp.c | 13 - > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/net/slirp.c b/net/slirp.c > > index 01ed21c006..d87664d42e 100644 > > --- a/net/slirp.c > > +++ b/net/slirp.c > > @@ -496,9 +496,11 @@ static int slirp_hostfwd(SlirpState *s, const char > > *redir_str, > > char buf[256]; > > int is_udp; > > char *end; > > +const char *fail_reason = ""; > > Isn't it better not initialize this? So if one add a new failed syntax case > the build with abort with -Werror=uninitialized > > Anyway: > Reviewed-by: Philippe Mathieu-Daudé Applied to my tree, thanks! (I have put Unknown reason there, otherwise the message would have oddly-looking "()" ) Samuel
Re: [Qemu-devel] [PATCH] slirp: fix clearing ifq_so from pending packets
Thomas Huth, on mer. 30 août 2017 09:50:45 +0200, wrote:
> On 26.08.2017 00:37, Samuel Thibault wrote:
> > The if_fastq and if_batchq contain not only packets, but queues of packets
> > for the same socket. When sofree frees a socket, it thus has to clear ifq_so
> > from all the packets from the queues, not only the first.
>
> I think you should CC: this to qemu-stable if it's fixing a problem that
> can be used by the guest to crash QEMU... ?
Indeed. I thought it should first go to master.
Samuel
> Thomas
>
> > Signed-off-by: Samuel Thibault
> > Acked-by: Philippe Mathieu-Daudé
> > ---
> > slirp/socket.c | 39 +++
> > 1 file changed, 23 insertions(+), 16 deletions(-)
> >
> > diff --git a/slirp/socket.c b/slirp/socket.c
> > index ecec0295a9..cb7b5b608d 100644
> > --- a/slirp/socket.c
> > +++ b/slirp/socket.c
> > @@ -59,6 +59,27 @@ socreate(Slirp *slirp)
> >return(so);
> > }
> >
> > +/*
> > + * Remove references to so from the given message queue.
> > + */
> > +static void
> > +soqfree(struct socket *so, struct quehead *qh)
> > +{
> > +struct mbuf *ifq;
> > +
> > +for (ifq = (struct mbuf *) qh->qh_link;
> > + (struct quehead *) ifq != qh;
> > + ifq = ifq->ifq_next) {
> > +if (ifq->ifq_so == so) {
> > +struct mbuf *ifm;
> > +ifq->ifq_so = NULL;
> > +for (ifm = ifq->ifs_next; ifm != ifq; ifm = ifm->ifs_next) {
> > +ifm->ifq_so = NULL;
> > +}
> > +}
> > +}
> > +}
> > +
> > /*
> > * remque and free a socket, clobber cache
> > */
> > @@ -66,23 +87,9 @@ void
> > sofree(struct socket *so)
> > {
> >Slirp *slirp = so->slirp;
> > - struct mbuf *ifm;
> >
> > - for (ifm = (struct mbuf *) slirp->if_fastq.qh_link;
> > - (struct quehead *) ifm != &slirp->if_fastq;
> > - ifm = ifm->ifq_next) {
> > -if (ifm->ifq_so == so) {
> > - ifm->ifq_so = NULL;
> > -}
> > - }
> > -
> > - for (ifm = (struct mbuf *) slirp->if_batchq.qh_link;
> > - (struct quehead *) ifm != &slirp->if_batchq;
> > - ifm = ifm->ifq_next) {
> > -if (ifm->ifq_so == so) {
> > - ifm->ifq_so = NULL;
> > -}
> > - }
> > + soqfree(so, &slirp->if_fastq);
> > + soqfree(so, &slirp->if_batchq);
> >
> >if (so->so_emu==EMU_RSH && so->extra) {
> > sofree(so->extra);
> >
>
--
Samuel
CN > J'ai enseigné l'algorythmique.
GLG> C'est quoi l'algorythmique ? Une contrebasse programmée en Algol ?
-+- in : Guide du Neuneu d'Usenet - Neuneu fait ses gammes. -+-
Re: [Qemu-devel] [PULL 0/1] slirp updates
Peter Maydell, on mar. 29 août 2017 16:31:03 +0100, wrote: > On 29 August 2017 at 10:22, Samuel Thibault wrote: > > Peter Maydell, on mar. 29 août 2017 10:20:31 +0100, wrote: > >> On 28 August 2017 at 00:05, Samuel Thibault > >> wrote: > >> > warning: redirection vers https://people.debian.org/~sthibault/qemu.git/ > >> > The following changes since commit > >> > 04d74e07b4542aad5aa4ad03951b38b767f5314a: > >> > > >> > slirp: fix clearing ifq_so from pending packets (2017-08-26 01:04:12 > >> > +0200) > >> > > >> > are available in the git repository at: > >> > > >> > http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault > >> > > >> > for you to fetch changes up to 04d74e07b4542aad5aa4ad03951b38b767f5314a: > >> > > >> > slirp: fix clearing ifq_so from pending packets (2017-08-26 01:04:12 > >> > +0200) > >> > > >> > > >> > slirp updates > >> > > >> > > >> > >> Is this pull request intended to be for 2.10 (in which > >> case it needs justification) or for 2.11 (in which case > >> it's a bit early) ? > > > > It is for 2.10. It fixes at least a DOS: userland can at least crash > > qemu with specially-crafted packets. > > It's really really hard to justify putting this in now. Alright, it's up to the release managers to decide anyway. Samuel
Re: [Qemu-devel] [PULL 0/1] slirp updates
Peter Maydell, on mar. 29 août 2017 10:20:31 +0100, wrote: > On 28 August 2017 at 00:05, Samuel Thibault > wrote: > > warning: redirection vers https://people.debian.org/~sthibault/qemu.git/ > > The following changes since commit 04d74e07b4542aad5aa4ad03951b38b767f5314a: > > > > slirp: fix clearing ifq_so from pending packets (2017-08-26 01:04:12 > > +0200) > > > > are available in the git repository at: > > > > http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault > > > > for you to fetch changes up to 04d74e07b4542aad5aa4ad03951b38b767f5314a: > > > > slirp: fix clearing ifq_so from pending packets (2017-08-26 01:04:12 > > +0200) > > > > > > slirp updates > > > > > > Is this pull request intended to be for 2.10 (in which > case it needs justification) or for 2.11 (in which case > it's a bit early) ? It is for 2.10. It fixes at least a DOS: userland can at least crash qemu with specially-crafted packets. Samuel
Re: [Qemu-devel] [PULL 1/1] slirp: fix clearing ifq_so from pending packets
Samuel Thibault, on lun. 28 août 2017 17:44:55 +0200, wrote: > Eric Blake, on lun. 28 août 2017 10:43:48 -0500, wrote: > > This is a pull request, but missed 2.10-rc4, which makes it really hard > > to justify for inclusion in 2.10 proper. Is it okay that this doesn't > > go in until 2.11? > > Well, AIUI we have been living with the issue for years (decades?), so > it can probably wait a bit more :) (OTOH, it's a at least DOS issue, userland can crash qemu with specially-crafted packets). Samuel
Re: [Qemu-devel] [PULL 1/1] slirp: fix clearing ifq_so from pending packets
Hello, Eric Blake, on lun. 28 août 2017 10:43:48 -0500, wrote: > This is a pull request, but missed 2.10-rc4, which makes it really hard > to justify for inclusion in 2.10 proper. Is it okay that this doesn't > go in until 2.11? Well, AIUI we have been living with the issue for years (decades?), so it can probably wait a bit more :) Samuel
[Qemu-devel] [PULL 0/1] slirp updates
warning: redirection vers https://people.debian.org/~sthibault/qemu.git/ The following changes since commit 04d74e07b4542aad5aa4ad03951b38b767f5314a: slirp: fix clearing ifq_so from pending packets (2017-08-26 01:04:12 +0200) are available in the git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 04d74e07b4542aad5aa4ad03951b38b767f5314a: slirp: fix clearing ifq_so from pending packets (2017-08-26 01:04:12 +0200) slirp updates
[Qemu-devel] [PULL 1/1] slirp: fix clearing ifq_so from pending packets
The if_fastq and if_batchq contain not only packets, but queues of packets
for the same socket. When sofree frees a socket, it thus has to clear ifq_so
from all the packets from the queues, not only the first.
Signed-off-by: Samuel Thibault
Reviewed-by: Philippe Mathieu-Daudé
---
slirp/socket.c | 39 +++
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/slirp/socket.c b/slirp/socket.c
index ecec0295a9..cb7b5b608d 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -59,6 +59,27 @@ socreate(Slirp *slirp)
return(so);
}
+/*
+ * Remove references to so from the given message queue.
+ */
+static void
+soqfree(struct socket *so, struct quehead *qh)
+{
+struct mbuf *ifq;
+
+for (ifq = (struct mbuf *) qh->qh_link;
+ (struct quehead *) ifq != qh;
+ ifq = ifq->ifq_next) {
+if (ifq->ifq_so == so) {
+struct mbuf *ifm;
+ifq->ifq_so = NULL;
+for (ifm = ifq->ifs_next; ifm != ifq; ifm = ifm->ifs_next) {
+ifm->ifq_so = NULL;
+}
+}
+}
+}
+
/*
* remque and free a socket, clobber cache
*/
@@ -66,23 +87,9 @@ void
sofree(struct socket *so)
{
Slirp *slirp = so->slirp;
- struct mbuf *ifm;
- for (ifm = (struct mbuf *) slirp->if_fastq.qh_link;
- (struct quehead *) ifm != &slirp->if_fastq;
- ifm = ifm->ifq_next) {
-if (ifm->ifq_so == so) {
- ifm->ifq_so = NULL;
-}
- }
-
- for (ifm = (struct mbuf *) slirp->if_batchq.qh_link;
- (struct quehead *) ifm != &slirp->if_batchq;
- ifm = ifm->ifq_next) {
-if (ifm->ifq_so == so) {
- ifm->ifq_so = NULL;
-}
- }
+ soqfree(so, &slirp->if_fastq);
+ soqfree(so, &slirp->if_batchq);
if (so->so_emu==EMU_RSH && so->extra) {
sofree(so->extra);
--
2.14.1
Re: [Qemu-devel] [PATCH] slirp: fix clearing ifq_so from pending packets
Samuel Thibault, on sam. 26 août 2017 01:05:04 +0200, wrote: > So Wjjzhang and PJP, can you confirm that this fixes your uses? PJP, can you forward it to Wjjzhang? I keep getting : host cloudmx.qq.com[113.108.11.188] said: 550 Mail content denied. http://ascloud.qq.com/cgi-bin/readtemplate?t=anti_spam_errors#n1000726 (in reply to end of DATA command) Samuel
Re: [Qemu-devel] [PATCH] slirp: fix clearing ifq_so from pending packets
Hello,
So Wjjzhang and PJP, can you confirm that this fixes your uses?
Samuel
Samuel Thibault, on sam. 26 août 2017 00:37:21 +0200, wrote:
> The if_fastq and if_batchq contain not only packets, but queues of packets
> for the same socket. When sofree frees a socket, it thus has to clear ifq_so
> from all the packets from the queues, not only the first.
>
> Signed-off-by: Samuel Thibault
> Acked-by: Philippe Mathieu-Daudé
> ---
> slirp/socket.c | 39 +++
> 1 file changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/slirp/socket.c b/slirp/socket.c
> index ecec0295a9..cb7b5b608d 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -59,6 +59,27 @@ socreate(Slirp *slirp)
>return(so);
> }
>
> +/*
> + * Remove references to so from the given message queue.
> + */
> +static void
> +soqfree(struct socket *so, struct quehead *qh)
> +{
> +struct mbuf *ifq;
> +
> +for (ifq = (struct mbuf *) qh->qh_link;
> + (struct quehead *) ifq != qh;
> + ifq = ifq->ifq_next) {
> +if (ifq->ifq_so == so) {
> +struct mbuf *ifm;
> +ifq->ifq_so = NULL;
> +for (ifm = ifq->ifs_next; ifm != ifq; ifm = ifm->ifs_next) {
> +ifm->ifq_so = NULL;
> +}
> +}
> +}
> +}
> +
> /*
> * remque and free a socket, clobber cache
> */
> @@ -66,23 +87,9 @@ void
> sofree(struct socket *so)
> {
>Slirp *slirp = so->slirp;
> - struct mbuf *ifm;
>
> - for (ifm = (struct mbuf *) slirp->if_fastq.qh_link;
> - (struct quehead *) ifm != &slirp->if_fastq;
> - ifm = ifm->ifq_next) {
> -if (ifm->ifq_so == so) {
> - ifm->ifq_so = NULL;
> -}
> - }
> -
> - for (ifm = (struct mbuf *) slirp->if_batchq.qh_link;
> - (struct quehead *) ifm != &slirp->if_batchq;
> - ifm = ifm->ifq_next) {
> -if (ifm->ifq_so == so) {
> - ifm->ifq_so = NULL;
> -}
> - }
> + soqfree(so, &slirp->if_fastq);
> + soqfree(so, &slirp->if_batchq);
>
>if (so->so_emu==EMU_RSH && so->extra) {
> sofree(so->extra);
> --
> 2.14.1
>
--
Samuel
...
et Ctrl alt F2 pour aller sous console
mais c koi pour passer d'un bureau a un autre !
au fait c koi le raccourci pour passer d'un bureau a un autre 'question
stupide"
ça dépend du window manager et de ta conf
ce qui fonctionne toujours c'est CTRL-ALT-BCKSP
-:- SignOff rv_: #linuxfr (Read error: EOF from client)
-:- rv_ [~rv@217.11.166.169] has joined #linuxfr
Firebird: MEURT...
[Qemu-devel] [PATCH] slirp: fix clearing ifq_so from pending packets
The if_fastq and if_batchq contain not only packets, but queues of packets
for the same socket. When sofree frees a socket, it thus has to clear ifq_so
from all the packets from the queues, not only the first.
Signed-off-by: Samuel Thibault
Acked-by: Philippe Mathieu-Daudé
---
slirp/socket.c | 39 +++
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/slirp/socket.c b/slirp/socket.c
index ecec0295a9..cb7b5b608d 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -59,6 +59,27 @@ socreate(Slirp *slirp)
return(so);
}
+/*
+ * Remove references to so from the given message queue.
+ */
+static void
+soqfree(struct socket *so, struct quehead *qh)
+{
+struct mbuf *ifq;
+
+for (ifq = (struct mbuf *) qh->qh_link;
+ (struct quehead *) ifq != qh;
+ ifq = ifq->ifq_next) {
+if (ifq->ifq_so == so) {
+struct mbuf *ifm;
+ifq->ifq_so = NULL;
+for (ifm = ifq->ifs_next; ifm != ifq; ifm = ifm->ifs_next) {
+ifm->ifq_so = NULL;
+}
+}
+}
+}
+
/*
* remque and free a socket, clobber cache
*/
@@ -66,23 +87,9 @@ void
sofree(struct socket *so)
{
Slirp *slirp = so->slirp;
- struct mbuf *ifm;
- for (ifm = (struct mbuf *) slirp->if_fastq.qh_link;
- (struct quehead *) ifm != &slirp->if_fastq;
- ifm = ifm->ifq_next) {
-if (ifm->ifq_so == so) {
- ifm->ifq_so = NULL;
-}
- }
-
- for (ifm = (struct mbuf *) slirp->if_batchq.qh_link;
- (struct quehead *) ifm != &slirp->if_batchq;
- ifm = ifm->ifq_next) {
-if (ifm->ifq_so == so) {
- ifm->ifq_so = NULL;
-}
- }
+ soqfree(so, &slirp->if_fastq);
+ soqfree(so, &slirp->if_batchq);
if (so->so_emu==EMU_RSH && so->extra) {
sofree(so->extra);
--
2.14.1
Re: [Qemu-devel] A use-after-free in slirp
Hello,
Thanks for the reproducer you sent me offline. Here is a fix which makes
a lot of sense and fixes the reproducer. Could you try it with your
whole testcase?
Could somebody also review the patch?
Samuel
commit 1a3a763509fad895c907e6978ea034a5c19ee370
Author: Samuel Thibault
Date: Fri Aug 25 01:35:53 2017 +0200
slirp: fix clearing ifq_so from pending packets
The if_fastq and if_batchq contain not only packets, but queues of packets
for the same socket. When sofree frees a socket, it thus has to clear ifq_so
from all the packets from the queues, not only the first.
Signed-off-by: Samuel Thibault
diff --git a/slirp/socket.c b/slirp/socket.c
index ecec0295a9..4203b80542 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -66,21 +66,29 @@ void
sofree(struct socket *so)
{
Slirp *slirp = so->slirp;
- struct mbuf *ifm;
-
- for (ifm = (struct mbuf *) slirp->if_fastq.qh_link;
- (struct quehead *) ifm != &slirp->if_fastq;
- ifm = ifm->ifq_next) {
-if (ifm->ifq_so == so) {
- ifm->ifq_so = NULL;
+ struct mbuf *ifq;
+
+ for (ifq = (struct mbuf *) slirp->if_fastq.qh_link;
+ (struct quehead *) ifq != &slirp->if_fastq;
+ ifq = ifq->ifq_next) {
+if (ifq->ifq_so == so) {
+ struct mbuf *ifm;
+ ifq->ifq_so = NULL;
+ for (ifm = ifq->ifs_next; ifm != ifq; ifm = ifm->ifs_next) {
+ifm->ifq_so = NULL;
+ }
}
}
- for (ifm = (struct mbuf *) slirp->if_batchq.qh_link;
- (struct quehead *) ifm != &slirp->if_batchq;
- ifm = ifm->ifq_next) {
-if (ifm->ifq_so == so) {
- ifm->ifq_so = NULL;
+ for (ifq = (struct mbuf *) slirp->if_batchq.qh_link;
+ (struct quehead *) ifq != &slirp->if_batchq;
+ ifq = ifq->ifq_next) {
+if (ifq->ifq_so == so) {
+ struct mbuf *ifm;
+ ifq->ifq_so = NULL;
+ for (ifm = ifq->ifs_next; ifm != ifq; ifm = ifm->ifs_next) {
+ifm->ifq_so = NULL;
+ }
}
}
Re: [Qemu-devel] A use-after-free in slirp
Hello, P J P, on jeu. 03 août 2017 17:45:06 +0530, wrote: > ==31922==ERROR: AddressSanitizer: heap-use-after-free on address > 0x6141ff8c at pc 0x56485de28ea0 bp 0x7f00f44fc950 sp 0x7f00f44fc940 > READ of size 4 at 0x6141ff8c thread T2 > #0 0x56485de28e9f in if_start slirp/if.c:230 > #1 0x56485de28a58 in if_output slirp/if.c:141 > #2 0x56485de35173 in ip_output slirp/ip_output.c:85 > #3 0x56485de57c48 in tcp_respond slirp/tcp_subr.c:218 > #4 0x56485de52440 in tcp_input slirp/tcp_input.c:1392 > #5 0x56485de329ef in ip_input slirp/ip_input.c:206 > #6 0x56485de3cf93 in slirp_input slirp/slirp.c:872 > #7 0x56485de0726d in net_slirp_receive net/slirp.c:119 > #8 0x56485ddee24d in nc_sendv_compat net/net.c:707 > #9 0x56485ddee3dd in qemu_deliver_packet_iov net/net.c:734 > #10 0x56485ddf422c in qemu_net_queue_deliver_iov net/queue.c:179 > ... Please don't strip the output :) The most interesting part is what exactly freed this. > A full trace output can be seen > > here -> https://paste.fedoraproject.org/paste/gh~hDctqUQ8uVt6UdG~zbg The paste is not available any more. Is it really very large? It's usually really better to just send it by mail, so it's archived in the mailing list etc. Samuel
Re: [Qemu-devel] SLIRP warning messages displayed while compiling
Eric Blake, on sam. 12 août 2017 06:41:30 -0500, wrote:
> Here's an idea: Instead of using struct ip6 { ... } QEMU_PACKED, use
>
> struct ip6 {
> ...
> };
> QEMU_BUG_ON(sizeof(struct ip6) != 32);
That's an interesting way indeed. I however wonder whether there may be
not-so-uncommon systems where the compiler aligns some fields just for
performance or such.
Samuel
Re: [Qemu-devel] SLIRP warning messages displayed while compiling
Peter Maydell, on sam. 12 août 2017 12:18:16 +0100, wrote: > On 12 August 2017 at 12:04, Samuel Thibault wrote: > > Peter Maydell, on sam. 12 août 2017 11:53:20 +0100, wrote: > >> The utility of the warning is that it means you get told > >> about stuff that might break on other architectures. > > > > Sure, I understand that. But here all fields are aligned on their size > > inside the packed structure. So there can't be alignment issues, and the > > compiler should be able to determine that. > > Alignment is architecture-dependent, and the compiler > can't know the alignment requirements for every > architecture. Sure. > There's no rule in C that says that uint16_t only needs 2 byte > alignment and not 4 on some hosts. Are there really systems where that happens? Such systems would have to pad uint16_t arrays then, really? > (Also I just noticed 'struct ip6' uses bitfields: > that's badly non-portable if it's trying to > match an on-the-wire layout.) struct ip has been doing so for so long too. I'm not saying the code is perfectly portable, I'm just saying it's portable enough for being both practical and readable. Samuel
Re: [Qemu-devel] SLIRP warning messages displayed while compiling
Peter Maydell, on sam. 12 août 2017 11:53:20 +0100, wrote: > The utility of the warning is that it means you get told > about stuff that might break on other architectures. Sure, I understand that. But here all fields are aligned on their size inside the packed structure. So there can't be alignment issues, and the compiler should be able to determine that. Samuel
Re: [Qemu-devel] SLIRP warning messages displayed while compiling
Hello, Programmingkid, on jeu. 10 août 2017 16:44:19 -0400, wrote: > While compiling I saw these error messages: > > slirp/ip6_icmp.c:79:32: warning: taking address of packed member 'ip_src' of > class or > structure 'ip6' may result in an unaligned pointer value > [-Waddress-of-packed-member] > if (IN6_IS_ADDR_MULTICAST(&ip->ip_src) || >^~ > slirp/ip_input.c:159:43: warning: taking address of packed member 'ip_link' > of class or > structure 'ipq' may result in an unaligned pointer value > [-Waddress-of-packed-member] > for (l = slirp->ipq.ip_link.next; l != &slirp->ipq.ip_link; > ^~ Well, if the compiler was counting well, it would notice that the structure are made so that even with packing, fields always get aligned on proper bounds. (and no, we don't want to remove QEMU_PACKED from struct ip6, at least, since it *has* to be packed to match the ipv6 headers). Samuel
[Qemu-devel] [PULL 0/2] slirp updates
warning: redirection vers https://people.debian.org/~sthibault/qemu.git/ The following changes since commit aaaec6acad7cf97372d48c1b09126a09697519c8: Update version for v2.10.0-rc1 release (2017-08-02 16:36:32 +0100) are available in the git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 413d463f43fbc4dd3a601e80a5724aa384a265a0: slirp: check len against dhcp options array end (2017-08-03 00:26:44 +0200) slirp updates Hervé Poussineau (1): slirp: fill error when failing to initialize user network Prasad J Pandit (1): slirp: check len against dhcp options array end net/slirp.c | 134 -- slirp/bootp.c | 3 ++ 2 files changed, 97 insertions(+), 40 deletions(-)
Re: [Qemu-devel] [PULL 2/2] slirp: check len against dhcp options array end
Samuel Thibault, on jeu. 03 août 2017 00:26:07 +0200, wrote:
> From: Prasad J Pandit
>
> While parsing dhcp options string in 'dhcp_decode', if an options'
> length 'len' appeared towards the end of 'bp_vend' array, ensuing
> read could lead to an OOB memory access issue. Add check to avoid it.
>
> Reported-by: Reno Robert
> Signed-off-by: Prasad J Pandit
> Signed-off-by: Samuel Thibault
Oops, this should have mentioned the CVE, I'll redo the pull request,
sorry.
> ---
> slirp/bootp.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/slirp/bootp.c b/slirp/bootp.c
> index 5a4646c182..5dd1a415b5 100644
> --- a/slirp/bootp.c
> +++ b/slirp/bootp.c
> @@ -123,6 +123,9 @@ static void dhcp_decode(const struct bootp_t *bp, int
> *pmsg_type,
> if (p >= p_end)
> break;
> len = *p++;
> +if (p + len > p_end) {
> +break;
> +}
> DPRINTF("dhcp: tag=%d len=%d\n", tag, len);
>
> switch(tag) {
> --
> 2.13.2
>
--
Samuel
"...[Linux's] capacity to talk via any medium except smoke signals."
(By Dr. Greg Wettstein, Roger Maris Cancer Center)
[Qemu-devel] [PULL 1/2] slirp: fill error when failing to initialize user network
From: Hervé Poussineau
With "-netdev user,id=net0,dns=1.2.3.4"
error was:
qemu-system-i386: -netdev user,id=net0,dns=1.2.3.4: Device 'user' could not be
initialized
Error is now:
qemu-system-i386: -netdev user,id=net0,dns=1.2.3.4: DNS doesn't belong to
network
Signed-off-by: Hervé Poussineau
Signed-off-by: Samuel Thibault
---
net/slirp.c | 134 ++--
1 file changed, 94 insertions(+), 40 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index 9fbc949e81..01ed21c006 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -91,15 +91,15 @@ static QTAILQ_HEAD(slirp_stacks, SlirpState) slirp_stacks =
QTAILQ_HEAD_INITIALIZER(slirp_stacks);
static int slirp_hostfwd(SlirpState *s, const char *redir_str,
- int legacy_format);
+ int legacy_format, Error **errp);
static int slirp_guestfwd(SlirpState *s, const char *config_str,
- int legacy_format);
+ int legacy_format, Error **errp);
#ifndef _WIN32
static const char *legacy_smb_export;
static int slirp_smb(SlirpState *s, const char *exported_dir,
- struct in_addr vserver_addr);
+ struct in_addr vserver_addr, Error **errp);
static void slirp_smb_cleanup(SlirpState *s);
#else
static inline void slirp_smb_cleanup(SlirpState *s) { }
@@ -155,7 +155,7 @@ static int net_slirp_init(NetClientState *peer, const char
*model,
const char *bootfile, const char *vdhcp_start,
const char *vnameserver, const char *vnameserver6,
const char *smb_export, const char *vsmbserver,
- const char **dnssearch)
+ const char **dnssearch, Error **errp)
{
/* default settings according to historic slirp */
struct in_addr net = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
@@ -178,15 +178,18 @@ static int net_slirp_init(NetClientState *peer, const
char *model,
struct slirp_config_str *config;
if (!ipv4 && (vnetwork || vhost || vnameserver)) {
+error_setg(errp, "IPv4 disabled but netmask/host/dns provided");
return -1;
}
if (!ipv6 && (vprefix6 || vhost6 || vnameserver6)) {
+error_setg(errp, "IPv6 disabled but prefix/host6/dns6 provided");
return -1;
}
if (!ipv4 && !ipv6) {
/* It doesn't make sense to disable both */
+error_setg(errp, "IPv4 and IPv6 disabled");
return -1;
}
@@ -200,6 +203,7 @@ static int net_slirp_init(NetClientState *peer, const char
*model,
if (vnetwork) {
if (get_str_sep(buf, sizeof(buf), &vnetwork, '/') < 0) {
if (!inet_aton(vnetwork, &net)) {
+error_setg(errp, "Failed to parse netmask");
return -1;
}
addr = ntohl(net.s_addr);
@@ -220,14 +224,19 @@ static int net_slirp_init(NetClientState *peer, const
char *model,
}
} else {
if (!inet_aton(buf, &net)) {
+error_setg(errp, "Failed to parse netmask");
return -1;
}
shift = strtol(vnetwork, &end, 10);
if (*end != '\0') {
if (!inet_aton(vnetwork, &mask)) {
+error_setg(errp,
+ "Failed to parse netmask (trailing chars)");
return -1;
}
} else if (shift < 4 || shift > 32) {
+error_setg(errp,
+ "Invalid netmask provided (must be in range 4-32)");
return -1;
} else {
mask.s_addr = htonl(0x << (32 - shift));
@@ -240,30 +249,43 @@ static int net_slirp_init(NetClientState *peer, const
char *model,
}
if (vhost && !inet_aton(vhost, &host)) {
+error_setg(errp, "Failed to parse host");
return -1;
}
if ((host.s_addr & mask.s_addr) != net.s_addr) {
+error_setg(errp, "Host doesn't belong to network");
return -1;
}
if (vnameserver && !inet_aton(vnameserver, &dns)) {
+error_setg(errp, "Failed to parse DNS");
return -1;
}
-if ((dns.s_addr & mask.s_addr) != net.s_addr ||
-dns.s_addr == host.s_addr) {
+if ((dns.s_addr & mask.s_addr) != net.s_addr) {
+error_setg(errp, "DNS doesn't belong to network");
+return -1;
+}
+if (dns.s_addr == host.s_addr) {
+error_setg(errp, "DNS must be different from host");
return -1;
}
if (vdhcp_start && !inet_aton(vdhc
[Qemu-devel] [PULL 2/2] slirp: check len against dhcp options array end
From: Prasad J Pandit
While parsing dhcp options string in 'dhcp_decode', if an options'
length 'len' appeared towards the end of 'bp_vend' array, ensuing
read could lead to an OOB memory access issue. Add check to avoid it.
This is CVE-2017-11434.
Reported-by: Reno Robert
Signed-off-by: Prasad J Pandit
Signed-off-by: Samuel Thibault
---
slirp/bootp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 5a4646c182..5dd1a415b5 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -123,6 +123,9 @@ static void dhcp_decode(const struct bootp_t *bp, int
*pmsg_type,
if (p >= p_end)
break;
len = *p++;
+if (p + len > p_end) {
+break;
+}
DPRINTF("dhcp: tag=%d len=%d\n", tag, len);
switch(tag) {
--
2.13.2
[Qemu-devel] [PULL 1/2] slirp: fill error when failing to initialize user network
From: Hervé Poussineau
With "-netdev user,id=net0,dns=1.2.3.4"
error was:
qemu-system-i386: -netdev user,id=net0,dns=1.2.3.4: Device 'user' could not be
initialized
Error is now:
qemu-system-i386: -netdev user,id=net0,dns=1.2.3.4: DNS doesn't belong to
network
Signed-off-by: Hervé Poussineau
Signed-off-by: Samuel Thibault
---
net/slirp.c | 134 ++--
1 file changed, 94 insertions(+), 40 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index 9fbc949e81..01ed21c006 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -91,15 +91,15 @@ static QTAILQ_HEAD(slirp_stacks, SlirpState) slirp_stacks =
QTAILQ_HEAD_INITIALIZER(slirp_stacks);
static int slirp_hostfwd(SlirpState *s, const char *redir_str,
- int legacy_format);
+ int legacy_format, Error **errp);
static int slirp_guestfwd(SlirpState *s, const char *config_str,
- int legacy_format);
+ int legacy_format, Error **errp);
#ifndef _WIN32
static const char *legacy_smb_export;
static int slirp_smb(SlirpState *s, const char *exported_dir,
- struct in_addr vserver_addr);
+ struct in_addr vserver_addr, Error **errp);
static void slirp_smb_cleanup(SlirpState *s);
#else
static inline void slirp_smb_cleanup(SlirpState *s) { }
@@ -155,7 +155,7 @@ static int net_slirp_init(NetClientState *peer, const char
*model,
const char *bootfile, const char *vdhcp_start,
const char *vnameserver, const char *vnameserver6,
const char *smb_export, const char *vsmbserver,
- const char **dnssearch)
+ const char **dnssearch, Error **errp)
{
/* default settings according to historic slirp */
struct in_addr net = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
@@ -178,15 +178,18 @@ static int net_slirp_init(NetClientState *peer, const
char *model,
struct slirp_config_str *config;
if (!ipv4 && (vnetwork || vhost || vnameserver)) {
+error_setg(errp, "IPv4 disabled but netmask/host/dns provided");
return -1;
}
if (!ipv6 && (vprefix6 || vhost6 || vnameserver6)) {
+error_setg(errp, "IPv6 disabled but prefix/host6/dns6 provided");
return -1;
}
if (!ipv4 && !ipv6) {
/* It doesn't make sense to disable both */
+error_setg(errp, "IPv4 and IPv6 disabled");
return -1;
}
@@ -200,6 +203,7 @@ static int net_slirp_init(NetClientState *peer, const char
*model,
if (vnetwork) {
if (get_str_sep(buf, sizeof(buf), &vnetwork, '/') < 0) {
if (!inet_aton(vnetwork, &net)) {
+error_setg(errp, "Failed to parse netmask");
return -1;
}
addr = ntohl(net.s_addr);
@@ -220,14 +224,19 @@ static int net_slirp_init(NetClientState *peer, const
char *model,
}
} else {
if (!inet_aton(buf, &net)) {
+error_setg(errp, "Failed to parse netmask");
return -1;
}
shift = strtol(vnetwork, &end, 10);
if (*end != '\0') {
if (!inet_aton(vnetwork, &mask)) {
+error_setg(errp,
+ "Failed to parse netmask (trailing chars)");
return -1;
}
} else if (shift < 4 || shift > 32) {
+error_setg(errp,
+ "Invalid netmask provided (must be in range 4-32)");
return -1;
} else {
mask.s_addr = htonl(0x << (32 - shift));
@@ -240,30 +249,43 @@ static int net_slirp_init(NetClientState *peer, const
char *model,
}
if (vhost && !inet_aton(vhost, &host)) {
+error_setg(errp, "Failed to parse host");
return -1;
}
if ((host.s_addr & mask.s_addr) != net.s_addr) {
+error_setg(errp, "Host doesn't belong to network");
return -1;
}
if (vnameserver && !inet_aton(vnameserver, &dns)) {
+error_setg(errp, "Failed to parse DNS");
return -1;
}
-if ((dns.s_addr & mask.s_addr) != net.s_addr ||
-dns.s_addr == host.s_addr) {
+if ((dns.s_addr & mask.s_addr) != net.s_addr) {
+error_setg(errp, "DNS doesn't belong to network");
+return -1;
+}
+if (dns.s_addr == host.s_addr) {
+error_setg(errp, "DNS must be different from host");
return -1;
}
if (vdhcp_start && !inet_aton(vdhc
[Qemu-devel] [PULL 0/2] slirp updates
warning: redirection vers https://people.debian.org/~sthibault/qemu.git/ The following changes since commit aaaec6acad7cf97372d48c1b09126a09697519c8: Update version for v2.10.0-rc1 release (2017-08-02 16:36:32 +0100) are available in the git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 2fe00b96b750f8c7a48dd3a98c5cb7370947264e: slirp: check len against dhcp options array end (2017-08-03 00:24:31 +0200) slirp updates Hervé Poussineau (1): slirp: fill error when failing to initialize user network Prasad J Pandit (1): slirp: check len against dhcp options array end net/slirp.c | 134 -- slirp/bootp.c | 3 ++ 2 files changed, 97 insertions(+), 40 deletions(-)
[Qemu-devel] [PULL 2/2] slirp: check len against dhcp options array end
From: Prasad J Pandit
While parsing dhcp options string in 'dhcp_decode', if an options'
length 'len' appeared towards the end of 'bp_vend' array, ensuing
read could lead to an OOB memory access issue. Add check to avoid it.
Reported-by: Reno Robert
Signed-off-by: Prasad J Pandit
Signed-off-by: Samuel Thibault
---
slirp/bootp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 5a4646c182..5dd1a415b5 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -123,6 +123,9 @@ static void dhcp_decode(const struct bootp_t *bp, int
*pmsg_type,
if (p >= p_end)
break;
len = *p++;
+if (p + len > p_end) {
+break;
+}
DPRINTF("dhcp: tag=%d len=%d\n", tag, len);
switch(tag) {
--
2.13.2
Re: [Qemu-devel] [PATCH] slirp: check len against dhcp options array end
Reno Robert, on lun. 17 juil. 2017 23:10:02 +0530, wrote:
> +if (p + len > p_end) {
>
> Shouldn't this be (p + len >= p_end) ?
No: if p_end-p is 1, len being 1 is fine.
Samuel
Re: [Qemu-devel] [PATCH] slirp: check len against dhcp options array end
P J P, on lun. 17 juil. 2017 17:33:26 +0530, wrote: > From: Prasad J Pandit > > While parsing dhcp options string in 'dhcp_decode', if an options' > length 'len' appeared towards the end of 'bp_vend' array, ensuing > read could lead to an OOB memory access issue. Add check to avoid it. > > Reported-by: Reno Robert > Signed-off-by: Prasad J Pandit Oops, sure, applied to my tree, thanks! Samuel
Re: [Qemu-devel] [PATCH] slirp: fill error when failing to initialize user network
HEllo, Hervé Poussineau, on sam. 15 juil. 2017 18:43:50 +0200, wrote: > With "-netdev user,id=net0,dns=1.2.3.4" > error was: > qemu-system-i386: -netdev user,id=net0,dns=1.2.3.4: Device 'user' could not > be initialized > > Error is now: > qemu-system-i386: -netdev user,id=net0,dns=1.2.3.4: DNS doesn't belong to > network > > Signed-off-by: Hervé Poussineau Applied to my tree, thanks! Samuel
[Qemu-devel] [PULL 1/4] slirp: use DIV_ROUND_UP
From: Marc-André Lureau
I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra
Signed-off-by: Marc-André Lureau
Signed-off-by: Samuel Thibault
---
slirp/ip6.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/slirp/ip6.h b/slirp/ip6.h
index 0908855f0f..b1bea43b3c 100644
--- a/slirp/ip6.h
+++ b/slirp/ip6.h
@@ -57,9 +57,9 @@ static inline bool in6_equal_mach(const struct in6_addr *a,
const struct in6_addr *b,
int prefix_len)
{
-if (memcmp(&(a->s6_addr[(prefix_len + 7) / 8]),
- &(b->s6_addr[(prefix_len + 7) / 8]),
- 16 - (prefix_len + 7) / 8) != 0) {
+if (memcmp(&(a->s6_addr[DIV_ROUND_UP(prefix_len, 8)]),
+ &(b->s6_addr[DIV_ROUND_UP(prefix_len, 8)]),
+ 16 - DIV_ROUND_UP(prefix_len, 8)) != 0) {
return 0;
}
--
2.13.2
[Qemu-devel] [PULL 0/4] slirp updates
warning: redirection vers https://people.debian.org/~sthibault/qemu.git/ The following changes since commit 4871b51b9241b10f4fd8e04bbb21577886795e25: vmgenid-test: use boot-sector infrastructure (2017-07-14 17:03:03 +0100) are available in the git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 75cb298d905030fca897ea1d80e409c7f7e3e5ea: slirp: Handle error returns from sosendoob() (2017-07-15 14:28:25 +0200) slirp updates Marc-André Lureau (1): slirp: use DIV_ROUND_UP Peter Maydell (3): slirp: fork_exec(): Don't close() a negative number in fork_exec() slirp: Handle error returns from slirp_send() in sosendoob() slirp: Handle error returns from sosendoob() slirp/ip6.h| 6 +++--- slirp/misc.c | 4 +++- slirp/sbuf.c | 2 +- slirp/socket.c | 52 +++- 4 files changed, 42 insertions(+), 22 deletions(-)
[Qemu-devel] [PULL 3/4] slirp: Handle error returns from slirp_send() in sosendoob()
From: Peter Maydell
The code in sosendoob() assumes that slirp_send() always
succeeds, but it might return an OS error code (for instance
if the other end has disconnected). Catch these and return
the caller either -1 on error or the number of urgent bytes
actually written. (None of the callers check this return
value currently, though.)
Signed-off-by: Peter Maydell
Reviewed-by: Dr. David Alan Gilbert
Signed-off-by: Samuel Thibault
---
slirp/socket.c | 29 ++---
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/slirp/socket.c b/slirp/socket.c
index 3b49a69a93..a17caa9fa7 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -345,33 +345,40 @@ sosendoob(struct socket *so)
if (sb->sb_rptr < sb->sb_wptr) {
/* We can send it directly */
n = slirp_send(so, sb->sb_rptr, so->so_urgc, (MSG_OOB)); /*
|MSG_DONTWAIT)); */
- so->so_urgc -= n;
-
- DEBUG_MISC((dfd, " --- sent %d bytes urgent data, %d urgent
bytes left\n", n, so->so_urgc));
} else {
/*
* Since there's no sendv or sendtov like writev,
* we must copy all data to a linear buffer then
* send it all
*/
+ uint32_t urgc = so->so_urgc;
len = (sb->sb_data + sb->sb_datalen) - sb->sb_rptr;
- if (len > so->so_urgc) len = so->so_urgc;
+ if (len > urgc) {
+ len = urgc;
+ }
memcpy(buff, sb->sb_rptr, len);
- so->so_urgc -= len;
- if (so->so_urgc) {
+ urgc -= len;
+ if (urgc) {
n = sb->sb_wptr - sb->sb_data;
- if (n > so->so_urgc) n = so->so_urgc;
+ if (n > urgc) {
+ n = urgc;
+ }
memcpy((buff + len), sb->sb_data, n);
- so->so_urgc -= n;
len += n;
}
n = slirp_send(so, buff, len, (MSG_OOB)); /* |MSG_DONTWAIT)); */
+ }
+
#ifdef DEBUG
- if (n != len)
- DEBUG_ERROR((dfd, "Didn't send all data urgently X\n"));
+ if (n != len) {
+ DEBUG_ERROR((dfd, "Didn't send all data urgently X\n"));
+ }
#endif
- DEBUG_MISC((dfd, " ---2 sent %d bytes urgent data, %d urgent
bytes left\n", n, so->so_urgc));
+ if (n < 0) {
+ return n;
}
+ so->so_urgc -= n;
+ DEBUG_MISC((dfd, " ---2 sent %d bytes urgent data, %d urgent bytes
left\n", n, so->so_urgc));
sb->sb_cc -= n;
sb->sb_rptr += n;
--
2.13.2
[Qemu-devel] [PULL 4/4] slirp: Handle error returns from sosendoob()
From: Peter Maydell
sosendoob() can return a failure code, but all its callers ignore it.
This is OK in sbappend(), as the comment there states -- we will try
again later in sowrite(). Add a (void) cast to tell Coverity so.
In sowrite() we do need to check the return value -- we should handle
a write failure in sosendoob() the same way we handle a write failure
for the normal data.
Signed-off-by: Peter Maydell
Reviewed-by: Dr. David Alan Gilbert
Signed-off-by: Samuel Thibault
---
slirp/sbuf.c | 2 +-
slirp/socket.c | 23 +--
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/slirp/sbuf.c b/slirp/sbuf.c
index 10119d3ad5..912f235f65 100644
--- a/slirp/sbuf.c
+++ b/slirp/sbuf.c
@@ -91,7 +91,7 @@ sbappend(struct socket *so, struct mbuf *m)
if (so->so_urgc) {
sbappendsb(&so->so_rcv, m);
m_free(m);
- sosendoob(so);
+ (void)sosendoob(so);
return;
}
diff --git a/slirp/socket.c b/slirp/socket.c
index a17caa9fa7..ecec0295a9 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -404,7 +404,15 @@ sowrite(struct socket *so)
DEBUG_ARG("so = %p", so);
if (so->so_urgc) {
- sosendoob(so);
+ uint32_t expected = so->so_urgc;
+ if (sosendoob(so) < expected) {
+ /* Treat a short write as a fatal error too,
+* rather than continuing on and sending the urgent
+* data as if it were non-urgent and leaving the
+* so_urgc count wrong.
+*/
+ goto err_disconnected;
+ }
if (sb->sb_cc == 0)
return 0;
}
@@ -448,11 +456,7 @@ sowrite(struct socket *so)
return 0;
if (nn <= 0) {
- DEBUG_MISC((dfd, " --- sowrite disconnected, so->so_state = %x,
errno = %d\n",
- so->so_state, errno));
- sofcantsendmore(so);
- tcp_sockclosed(sototcpcb(so));
- return -1;
+ goto err_disconnected;
}
#ifndef HAVE_READV
@@ -479,6 +483,13 @@ sowrite(struct socket *so)
sofcantsendmore(so);
return nn;
+
+err_disconnected:
+ DEBUG_MISC((dfd, " --- sowrite disconnected, so->so_state = %x, errno =
%d\n",
+ so->so_state, errno));
+ sofcantsendmore(so);
+ tcp_sockclosed(sototcpcb(so));
+ return -1;
}
/*
--
2.13.2
[Qemu-devel] [PULL 2/4] slirp: fork_exec(): Don't close() a negative number in fork_exec()
From: Peter Maydell
In a fork_exec() error path we try to closesocket(s) when s might
be a negative number because the thing that failed was the
qemu_socket() call. Add a guard so we don't do this.
(Spotted by Coverity: CID 1005727 issue 1 of 2.)
Signed-off-by: Peter Maydell
Reviewed-by: Dr. David Alan Gilbert
Signed-off-by: Samuel Thibault
---
slirp/misc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/slirp/misc.c b/slirp/misc.c
index 88e9d94197..260187b6b6 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -112,7 +112,9 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
bind(s, (struct sockaddr *)&addr, addrlen) < 0 ||
listen(s, 1) < 0) {
error_report("Error: inet socket: %s", strerror(errno));
- closesocket(s);
+ if (s >= 0) {
+ closesocket(s);
+ }
return 0;
}
--
2.13.2
Re: [Qemu-devel] [PATCH v2 2/2] slirp: Handle error returns from sosendoob()
Dr. David Alan Gilbert, on ven. 14 juil. 2017 15:24:19 +0100, wrote: > * Peter Maydell (peter.mayd...@linaro.org) wrote: > > sosendoob() can return a failure code, but all its callers ignore it. > > This is OK in sbappend(), as the comment there states -- we will try > > again later in sowrite(). Add a (void) cast to tell Coverity so. > > In sowrite() we do need to check the return value -- we should handle > > a write failure in sosendoob() the same way we handle a write failure > > for the normal data. > > > > Signed-off-by: Peter Maydell > Reviewed-by: Dr. David Alan Gilbert Applied to my tree, thanks! Samuel
Re: [Qemu-devel] [1/2] slirp: Handle error returns from slirp_send() in sosendoob()
Peter Maydell, on lun. 05 juin 2017 17:19:35 +0100, wrote: > The code in sosendoob() assumes that slirp_send() always > succeeds, but it might return an OS error code (for instance > if the other end has disconnected). Catch these and return > the caller either -1 on error or the number of urgent bytes > actually written. (None of the callers check this return > value currently, though.) > > Signed-off-by: Peter Maydell > Reviewed-by: Dr. David Alan Gilbert Applied to my tree, thanks!
Re: [Qemu-devel] [2/2] slirp: Handle error returns from sosendoob()
Peter Maydell, on lun. 05 juin 2017 17:19:36 +0100, wrote:
> diff --git a/slirp/socket.c b/slirp/socket.c
> index a17caa9..84cf13a 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -404,7 +404,14 @@ sowrite(struct socket *so)
> DEBUG_ARG("so = %p", so);
>
> if (so->so_urgc) {
> - sosendoob(so);
> + if (sosendoob(so) < so->so_urgc) {
Mmm, I believe one needs to use a copy of so->so_urgc, since sosendoob()
modifies it in the success case?
Samuel
Re: [Qemu-devel] [PATCH] slirp: fork_exec(): Don't close() a negative number in fork_exec()
Peter Maydell, on dim. 09 juil. 2017 18:54:22 +0100, wrote: > In a fork_exec() error path we try to closesocket(s) when s might > be a negative number because the thing that failed was the > qemu_socket() call. Add a guard so we don't do this. > > (Spotted by Coverity: CID 1005727 issue 1 of 2.) Applied to my tree, thanks! Samuel
Re: [Qemu-devel] [PATCH 0/2] slirp: handle errors in sosendoob()
Peter Maydell, on dim. 09 juil. 2017 22:21:01 +0100, wrote: > Ping^2 ? I'm sorry I'm still too busy ATM, it's still far in my mbox. Samuel
Re: [Qemu-devel] [PATCH 12/31] slirp: use DIV_ROUND_UP
Marc-André Lureau, on jeu. 22 juin 2017 14:41:45 +0200, wrote:
> I used the clang-tidy qemu-round check to generate the fix:
> https://github.com/elmarco/clang-tools-extra
>
> Signed-off-by: Marc-André Lureau
Applied to my tree, thanks!
> ---
> slirp/ip6.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/slirp/ip6.h b/slirp/ip6.h
> index 0908855f0f..b1bea43b3c 100644
> --- a/slirp/ip6.h
> +++ b/slirp/ip6.h
> @@ -57,9 +57,9 @@ static inline bool in6_equal_mach(const struct in6_addr *a,
>const struct in6_addr *b,
>int prefix_len)
> {
> -if (memcmp(&(a->s6_addr[(prefix_len + 7) / 8]),
> - &(b->s6_addr[(prefix_len + 7) / 8]),
> - 16 - (prefix_len + 7) / 8) != 0) {
> +if (memcmp(&(a->s6_addr[DIV_ROUND_UP(prefix_len, 8)]),
> + &(b->s6_addr[DIV_ROUND_UP(prefix_len, 8)]),
> + 16 - DIV_ROUND_UP(prefix_len, 8)) != 0) {
> return 0;
> }
>
> --
> 2.13.1.395.gf7b71de06
>
--
Samuel
N: j'aime bien Cut d'un truc enorme... ca montre de quel cote de l'ecran
sont les gonades :)))
-+- #ens-mim et la peufeupeu -+-
[Qemu-devel] [PULL 0/3] slirp updates
The following changes since commit 9964e96dccf7f7c936ee854a795415d19b60: Merge remote-tracking branch 'jasowang/tags/net-pull-request' into staging (2017-05-23 15:01:31 +0100) are available in the git repository at: http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault for you to fetch changes up to 2e30230aa95a2d6cfaadac015bd96c3db19c45e4: Fix total IP header length in forwarded TCP packets (2017-05-27 23:35:00 +0200) slirp updates Marc-André Lureau (1): slirp: fix leak Sjors Gielen (1): Fix total IP header length in forwarded TCP packets Tao Wu (1): slirp: Fix wrong mss bug. slirp/socket.c| 3 +++ slirp/tcp_input.c | 4 ++-- slirp/tcp_subr.c | 4 ++-- 3 files changed, 7 insertions(+), 4 deletions(-)
[Qemu-devel] [PULL 3/3] Fix total IP header length in forwarded TCP packets
From: Sjors Gielen When forwarding TCP packets, the internal tcpiphdr struct length was wrongly used inside the IP header. This commit changes the behaviour to what is used by tcp_output.c, using the correct full IP header + payload length. Signed-off-by: Sjors Gielen Signed-off-by: Samuel Thibault --- slirp/tcp_subr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c index ed16e1807f..dc8b4bbb50 100644 --- a/slirp/tcp_subr.c +++ b/slirp/tcp_subr.c @@ -204,7 +204,7 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti, struct mbuf *m, m->m_len -= sizeof(struct tcpiphdr) - sizeof(struct tcphdr) - sizeof(struct ip); ip = mtod(m, struct ip *); - ip->ip_len = tlen; + ip->ip_len = m->m_len; ip->ip_dst = tcpiph_save.ti_dst; ip->ip_src = tcpiph_save.ti_src; ip->ip_p = tcpiph_save.ti_pr; @@ -224,7 +224,7 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti, struct mbuf *m, m->m_len -= sizeof(struct tcpiphdr) - sizeof(struct tcphdr) - sizeof(struct ip6); ip6 = mtod(m, struct ip6 *); - ip6->ip_pl = tlen; + ip6->ip_pl = tcpiph_save.ti_len; ip6->ip_dst = tcpiph_save.ti_dst6; ip6->ip_src = tcpiph_save.ti_src6; ip6->ip_nh = tcpiph_save.ti_nh6; -- 2.11.0
[Qemu-devel] [PULL 1/3] slirp: Fix wrong mss bug.
From: Tao Wu
This bug was introduced by https://github.com/qemu/qemu/commit/98c6305
Signed-off-by: Tao Wu
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-bu: Samuel Thibault
---
slirp/tcp_input.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c
index edb98f06f3..07bcbdb2dd 100644
--- a/slirp/tcp_input.c
+++ b/slirp/tcp_input.c
@@ -1587,11 +1587,11 @@ tcp_mss(struct tcpcb *tp, u_int offer)
switch (so->so_ffamily) {
case AF_INET:
mss = MIN(IF_MTU, IF_MRU) - sizeof(struct tcphdr)
- + sizeof(struct ip);
+ - sizeof(struct ip);
break;
case AF_INET6:
mss = MIN(IF_MTU, IF_MRU) - sizeof(struct tcphdr)
- + sizeof(struct ip6);
+ - sizeof(struct ip6);
break;
default:
g_assert_not_reached();
--
2.11.0
[Qemu-devel] [PULL 2/3] slirp: fix leak
From: Marc-André Lureau
Spotted by ASAN:
/x86_64/hmp/pc-0.12:
=
==22538==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 224 byte(s) in 1 object(s) allocated from:
#0 0x7f0f63cdee60 in malloc (/lib64/libasan.so.3+0xc6e60)
#1 0x556f11ff32d7 in tcp_newtcpcb
/home/elmarco/src/qemu/slirp/tcp_subr.c:250
#2 0x556f11fdb1d1 in tcp_listen /home/elmarco/src/qemu/slirp/socket.c:688
#3 0x556f11fca9d5 in slirp_add_hostfwd
/home/elmarco/src/qemu/slirp/slirp.c:1052
#4 0x556f11f8db41 in slirp_hostfwd /home/elmarco/src/qemu/net/slirp.c:506
#5 0x556f11f8dd83 in hmp_hostfwd_add /home/elmarco/src/qemu/net/slirp.c:535
There might be a better way to fix this, but calling slirp tcp_close()
doesn't work.
Signed-off-by: Marc-André Lureau
Signed-off-by: Samuel Thibault
---
slirp/socket.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/slirp/socket.c b/slirp/socket.c
index 86927722e1..3b49a69a93 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -100,6 +100,9 @@ sofree(struct socket *so)
if(so->so_next && so->so_prev)
remque(so); /* crashes if so is not in a queue */
+ if (so->so_tcpcb) {
+ free(so->so_tcpcb);
+ }
free(so);
}
--
2.11.0
Re: [Qemu-devel] [PATCH] [SLIRP] Fix total IP header length in forwarded TCP packets
Sjors Gielen, on mer. 24 mai 2017 17:51:12 +, wrote: > When forwarding TCP packets, the internal tcpiphdr struct length was wrongly > used inside the IP header. This commit changes the behaviour to what is used > by tcp_output.c, using the correct full IP header + payload length. Indeed, applied, thanks. Samuel
Re: [Qemu-devel] [PATCH] migration: remove register_savevm()
Laurent Vivier, on mer. 24 mai 2017 14:10:48 +0200, wrote:
> We can replace the four remaining calls of register_savevm() by
> calls to register_savevm_live(). So we can remove the function and
> as we don't allocate anymore the ops pointer with g_new0()
> we don't have to free it then.
>
> Signed-off-by: Laurent Vivier
Acked-by: Samuel Thibault
> ---
> hw/net/vmxnet3.c| 8 ++--
> hw/s390x/s390-skeys.c | 9 +++--
> hw/s390x/s390-virtio-ccw.c | 8 ++--
> include/migration/vmstate.h | 8
> migration/savevm.c | 16
> slirp/slirp.c | 8 ++--
> 6 files changed, 25 insertions(+), 32 deletions(-)
>
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 8b1fab2..4df3110 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -2262,6 +2262,11 @@ static const MemoryRegionOps b1_ops = {
> },
> };
>
> +static SaveVMHandlers savevm_vmxnet3_msix = {
> +.save_state = vmxnet3_msix_save,
> +.load_state = vmxnet3_msix_load,
> +};
> +
> static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
> {
> uint64_t dsn_payload;
> @@ -2331,8 +2336,7 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev,
> Error **errp)
>vmxnet3_device_serial_num(s));
> }
>
> -register_savevm(dev, "vmxnet3-msix", -1, 1,
> -vmxnet3_msix_save, vmxnet3_msix_load, s);
> +register_savevm_live(dev, "vmxnet3-msix", -1, 1, &savevm_vmxnet3_msix,
> s);
> }
>
> static void vmxnet3_instance_init(Object *obj)
> diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
> index e2d4e1a..a3dc088 100644
> --- a/hw/s390x/s390-skeys.c
> +++ b/hw/s390x/s390-skeys.c
> @@ -363,6 +363,11 @@ static inline bool
> s390_skeys_get_migration_enabled(Object *obj, Error **errp)
> return ss->migration_enabled;
> }
>
> +static SaveVMHandlers savevm_s390_storage_keys = {
> +.save_state = s390_storage_keys_save,
> +.load_state = s390_storage_keys_load,
> +};
> +
> static inline void s390_skeys_set_migration_enabled(Object *obj, bool value,
> Error **errp)
> {
> @@ -376,8 +381,8 @@ static inline void
> s390_skeys_set_migration_enabled(Object *obj, bool value,
> ss->migration_enabled = value;
>
> if (ss->migration_enabled) {
> -register_savevm(NULL, TYPE_S390_SKEYS, 0, 1, s390_storage_keys_save,
> -s390_storage_keys_load, ss);
> +register_savevm_live(NULL, TYPE_S390_SKEYS, 0, 1,
> + &savevm_s390_storage_keys, ss);
> } else {
> unregister_savevm(DEVICE(ss), TYPE_S390_SKEYS, ss);
> }
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index fdd4384..697a2d6 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -104,6 +104,11 @@ void s390_memory_init(ram_addr_t mem_size)
> s390_skeys_init();
> }
>
> +static SaveVMHandlers savevm_gtod = {
> +.save_state = gtod_save,
> +.load_state = gtod_load,
> +};
> +
> static void ccw_init(MachineState *machine)
> {
> int ret;
> @@ -146,8 +151,7 @@ static void ccw_init(MachineState *machine)
> s390_create_virtio_net(BUS(css_bus), "virtio-net-ccw");
>
> /* Register savevm handler for guest TOD clock */
> -register_savevm(NULL, "todclock", 0, 1,
> -gtod_save, gtod_load, kvm_state);
> +register_savevm_live(NULL, "todclock", 0, 1, &savevm_gtod, kvm_state);
> }
>
> static void s390_cpu_plug(HotplugHandler *hotplug_dev,
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index f97411d..21aa6ca 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -59,14 +59,6 @@ typedef struct SaveVMHandlers {
> LoadStateHandler *load_state;
> } SaveVMHandlers;
>
> -int register_savevm(DeviceState *dev,
> -const char *idstr,
> -int instance_id,
> -int version_id,
> -SaveStateHandler *save_state,
> -LoadStateHandler *load_state,
> -void *opaque);
> -
> int register_savevm_live(DeviceState *dev,
> const char *idstr,
> int instance_id,
> diff --git a/migration/savevm.c b/migration/savevm.c
> index d971e5e..32badfc 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -617,21 +617,6 @@ int register_savevm_live(DeviceStat
Re: [Qemu-devel] [PATCH v2] tests/libqtest: Print error instead of aborting when env variable is missing
Thomas Huth, on mar. 23 mai 2017 09:25:30 +0200, wrote: > > /tmp/qemu-test/src/slirp/tcp_input.c: In function ‘tcp_input’: > > /tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_p’ may be > > used uninitialized in this function > > I've never seen these warnings in tcp_input.c before ... and they also > look like false positives too me ... They are. It's the classical false-positive: int foo; if (blah) foo = 1; ... if (blah) f(foo); where gcc doesn't realize it's the same condition between the initialization and the use. If that poses problem, we can force a dumb initialization... Samuel
Re: [Qemu-devel] [PATCH v5 0/1] slirp: add SOCKS5 support
Hello, Laurent Vivier, on mar. 09 mai 2017 21:31:11 +0200, wrote: > This patch implements the SOCKS5 client part for "-net user" backend. Thanks for the changes! It seems from the bot result that windows doesn't define in_port_t. Could you post a revised patch which uses sizeof(uint16_t) instead of sizeof(in_port_t), to trigger the bot again and confirm that this is the last needed fix for windows? Samuel
Re: [Qemu-devel] [PATCH v4 1/1] slirp: add SOCKS5 support
Hello,
Laurent Vivier, on mar. 09 mai 2017 09:21:09 +0200, wrote:
> Le 08/05/2017 à 22:09, Samuel Thibault a écrit :
> > Laurent Vivier, on sam. 06 mai 2017 15:19:44 +0200, wrote:
> >>> Laurent Vivier, on sam. 06 mai 2017 00:48:33 +0200, wrote:
> >> The check is done previously by:
> >>
> >> @@ -442,6 +443,10 @@ void slirp_pollfds_fill(GArray *pollfds, uint32_t
> >> *timeout)
> >> .fd = so->s,
> >> .events = G_IO_OUT | G_IO_ERR,
> >> };
> >> +if (so->so_proxy_state &&
> >> +so->so_proxy_state != SOCKS5_STATE_ERROR) {
> >> +pfd.events |= G_IO_IN;
> >> +}
> >>
> >> We can enter in socks5_recv() only if so->so_proxy_state is in a valid
> >> state because G_IO_IN triggers that.
> >
> > I don't understand: the socks5_recv gets called not only when pfd.events
> > contains G_IO_IN, but also when it contains G_IO_HUP or G_IO_ERR. Also,
> > so_proxy_state being non-nul is not the only way to have G_IO_IN set in
> > pfd.events, so_state & SS_FACCEPTCONN and CONN_CANFRCV(so) also trigger
> > that.
>
> This is filtered by (so_state & SS_ISFCONNECTING). The only case when we
> enter in the receiving part with SS_ISFCONNECTING is when socks5 part
> has been enabled.
The code snippet above is guarded by (so_state & SS_ISFCONNECTING),
but that won't prevent socks5_recv from being called here in
slirp_pollfds_poll in the non-socks5 case:
else if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
+if (so->so_state & SS_ISFCONNECTING) {
+socks5_recv(so->s, &so->so_proxy_state);
+continue;
+}
/*
e.g. when (so->so_state & SS_FACCEPTCONN) or when CONN_CANFRCV(so) in
slirp_pollfds_fill, which both set G_IO_IN too.
> >>>> @@ -645,11 +654,19 @@ void slirp_pollfds_poll(GArray *pollfds, int
> >>>> select_error)
> >>>> /*
> >>>> * Check for non-blocking, still-connecting sockets
> >>>> */
> >>>> -if (so->so_state & SS_ISFCONNECTING) {
> >>>> -/* Connected */
> >>>> -so->so_state &= ~SS_ISFCONNECTING;
> >>>>
> >>>> -ret = send(so->s, (const void *) &ret, 0, 0);
> >>>> +if (so->so_state & SS_ISFCONNECTING) {
> >>>> +ret = socks5_send(so->s, slirp->proxy_user,
> >>>
> >>> Ditto.
> >>
> >> if so_proxy_state is 0 the function does nothing
> >
> > Well, that's quite weak reason to me, and will confuse readers, because
> > they'll think that the code is for the socks5 case only.
>
> OK, I'm going to add a "if (so->so_proxy_state && so->so_proxy_state !=
> SOCKS5_STATE_ERROR)" here.
Thanks :)
> >>>> +++ b/slirp/socks5.c
> >>>> @@ -0,0 +1,371 @@
> >>>
> >>> In v2 of the patch, this was said to have "some parts from nmap/ncat
> >>> GPLv2". Is that really not true any more? If any part of the file is
> >>> not original, it *has* to wear proper copyright notices, otherwise it's
> >>> copyright infrigement.
> >>
> >> No, I've re-written entirely this part from RFC.
> >
> > Ok.
> >
> >> for ncat.h
> >
> > Which code comes from ncat.h?
>
> I haven't been clear: none. I've entirely re-written this part.
Ah, ok.
Samuel
Re: [Qemu-devel] [PATCH v4 1/1] slirp: add SOCKS5 support
Hello,
Laurent Vivier, on sam. 06 mai 2017 15:19:44 +0200, wrote:
> > Laurent Vivier, on sam. 06 mai 2017 00:48:33 +0200, wrote:
> >> @@ -617,6 +622,10 @@ void slirp_pollfds_poll(GArray *pollfds, int
> >> select_error)
> >> * Check sockets for reading
> >> */
> >> else if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
> >> +if (so->so_state & SS_ISFCONNECTING) {
> >> +socks5_recv(so->s, &so->so_proxy_state);
> >> +continue;
> >> +}
> >
> > Again, I don't see how this can work with both socks5 case and
> > non-socks5 case. Don't we need to somehow check for the type of socket
> > before calling socks5_recv?
>
> The check is done previously by:
>
> @@ -442,6 +443,10 @@ void slirp_pollfds_fill(GArray *pollfds, uint32_t
> *timeout)
> .fd = so->s,
> .events = G_IO_OUT | G_IO_ERR,
> };
> +if (so->so_proxy_state &&
> +so->so_proxy_state != SOCKS5_STATE_ERROR) {
> +pfd.events |= G_IO_IN;
> +}
>
> We can enter in socks5_recv() only if so->so_proxy_state is in a valid
> state because G_IO_IN triggers that.
I don't understand: the socks5_recv gets called not only when pfd.events
contains G_IO_IN, but also when it contains G_IO_HUP or G_IO_ERR. Also,
so_proxy_state being non-nul is not the only way to have G_IO_IN set in
pfd.events, so_state & SS_FACCEPTCONN and CONN_CANFRCV(so) also trigger
that.
> >> @@ -645,11 +654,19 @@ void slirp_pollfds_poll(GArray *pollfds, int
> >> select_error)
> >> /*
> >> * Check for non-blocking, still-connecting sockets
> >> */
> >> -if (so->so_state & SS_ISFCONNECTING) {
> >> -/* Connected */
> >> -so->so_state &= ~SS_ISFCONNECTING;
> >>
> >> -ret = send(so->s, (const void *) &ret, 0, 0);
> >> +if (so->so_state & SS_ISFCONNECTING) {
> >> +ret = socks5_send(so->s, slirp->proxy_user,
> >
> > Ditto.
>
> if so_proxy_state is 0 the function does nothing
Well, that's quite weak reason to me, and will confuse readers, because
they'll think that the code is for the socks5 case only.
> >> +++ b/slirp/socks5.c
> >> @@ -0,0 +1,371 @@
> >
> > In v2 of the patch, this was said to have "some parts from nmap/ncat
> > GPLv2". Is that really not true any more? If any part of the file is
> > not original, it *has* to wear proper copyright notices, otherwise it's
> > copyright infrigement.
>
> No, I've re-written entirely this part from RFC.
Ok.
> for ncat.h
Which code comes from ncat.h?
Again, we can't copy/paste code like that, that is copyright
infrigement.
> license is 281 lines long (with clarifications and
> extensions) for 60 lines copied...
That is really no reason. Copyright thingies are considered as soon
as dozen-ish lines of non-trivial code. The size of the terms of the
licence itself really doesn't matter. If we don't respect others'
copyright, we can't expect others (e.g. companies) to respect our GPL
code.
Samuel
Re: [Qemu-devel] [PATCH 1/1] slirp: don't zero ti_i since we acccess it later.
Hello, lepton, on lun. 08 mai 2017 12:08:55 -0700, wrote: > 1. For some reason, caller didn't setup anything in tcpiphdr, so there is > random data inside it. > 2. For some reason, caller setup correct src/dst address in tcpiphdr but > don't > zero ix_h1 > If you still think this doesn't look likely happen, Well, 1) is very likely to have been spotten another way. And crashing just because of 2) is harsh on users. The original code was setting ih_x1 at this point, so that was the original intent. > I am fine with your suggestion and will add zero for ih_x1, any > comments? Samuel
Re: [Qemu-devel] dns server not working in QEMU using usermode networking (SLIRP)
FONNEMANN Mark, on sam. 06 mai 2017 04:18:52 +, wrote:
> >That's so weird there's no packet log. Does your guest perhaps have the
> >strace tool? Or perhaps you >can manage to copy it over from another VM,
> >since it only depends on libc.
>
> See attached file.
Ok, so for some reason it doesn't even try to emit a packet...
> open("/etc/resolv.conf", O_RDONLY) = 3
> read(3, "nameserver 10.0.2.3\n\n", 4096) = 21
It does get the proper nameserver.
> connect(3, {sa_family=AF_FILE, path="/var/run/nscd/socket"}, 110) = -1 ENOENT
> (No such file or directory)
It tries to connect to nscd but that's not running.
> open("/etc/nsswitch.conf", O_RDONLY)= 3
> read(3, "hosts: files\n", 4096)= 18
> read(3, "", 4096) = 0
And I guess that's the culprit: in nsswitch.conf you only have
"hosts: files", so
> open("/lib/libnss_files.so.2", O_RDONLY) = 3
> open("/etc/hosts", O_RDONLY)= 3
> read(3, "127.0.0.1\tlocalhost\n10.0.2.15\tqe"..., 4096) = 35
It only uses /etc/hosts.
So, in your /etc/nsswitch.conf, use
hosts: files dns
Samuel
Re: [Qemu-devel] dns server not working in QEMU using usermode networking (SLIRP)
FONNEMANN Mark, on sam. 06 mai 2017 01:06:31 +, wrote: > root@qemu:~# /initrd/bin/nslookup www.google.com 10.0.2.3 > Server:10.0.2.3 > Address 1: 10.0.2.3 > > nslookup: can't resolve 'www.google.com' That's so weird there's no packet log. Does your guest perhaps have the strace tool? Or perhaps you can manage to copy it over from another VM, since it only depends on libc. > >Do you get output when pinging 10.0.2.3? > >(you won't get ping answers, but that's fine, qemu should still be able to > >see the requests coming in) > > root@qemu:~# ping 10.0.2.3 > PING 10.0.2.3 (10.0.2.3): 56 data bytes > > --- 10.0.2.3 ping statistics --- > 9 packets transmitted, 0 packets received, 100% packet > loss > > [mfonnemann@localhost qemu]$ cat logfile > ip input a00020f -> a000203 1 Ok, so at least something gets through, even if not UDP 53. Samuel
Re: [Qemu-devel] dns server not working in QEMU using usermode networking (SLIRP)
FONNEMANN Mark, on sam. 06 mai 2017 00:33:40 +, wrote: > >The attached patch should be dumping the same kind of information. > > Nslookup does not produce any output in curses or in logfile (-D logfile.txt). Uh. So qemu can't be the culprit since it doesn't receive anything :) > root@qemu:~# /initrd/bin/nslookup www.google.com > Server:10.0.2.3 > Address 1: 10.0.2.3 > > nslookup: can't resolve 'www.google.com' So it's supposed to be using 10.0.2.3 I guess. Just to make sure, you could try /initrd/bin/nslookup www.google.com 10.0.2.3 > I know the patch was applied because when I ping 10.0.2.2 I get the following: > > root@qemu:~# ping 10.0.2.2 > PING 10.0.2.2 (10.0.2.2): 56 data bytes > 64 bytes from 10.0.2.2: icmp_seq=0 ttl=255 time=0.5 ms > [mfonnemann@localhost qemu]$ cat logfile > ip input a00020f -> a000202 1 > ip input a00020f -> a000202 1 Do you get output when pinging 10.0.2.3? (you won't get ping answers, but that's fine, qemu should still be able to see the requests coming in) Also, just to make sure: you don't have firewalling rules, do you? Samuel
Re: [Qemu-devel] [PATCH v5 4/5] slirp: fix leak
Applied to my tree, thanks!
Re: [Qemu-devel] [PATCH v4 1/1] slirp: add SOCKS5 support
Hello,
Laurent Vivier, on sam. 06 mai 2017 00:48:33 +0200, wrote:
> @@ -617,6 +622,10 @@ void slirp_pollfds_poll(GArray *pollfds, int
> select_error)
> * Check sockets for reading
> */
> else if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
> +if (so->so_state & SS_ISFCONNECTING) {
> +socks5_recv(so->s, &so->so_proxy_state);
> +continue;
> +}
Again, I don't see how this can work with both socks5 case and
non-socks5 case. Don't we need to somehow check for the type of socket
before calling socks5_recv?
> @@ -645,11 +654,19 @@ void slirp_pollfds_poll(GArray *pollfds, int
> select_error)
> /*
> * Check for non-blocking, still-connecting sockets
> */
> -if (so->so_state & SS_ISFCONNECTING) {
> -/* Connected */
> -so->so_state &= ~SS_ISFCONNECTING;
>
> -ret = send(so->s, (const void *) &ret, 0, 0);
> +if (so->so_state & SS_ISFCONNECTING) {
> +ret = socks5_send(so->s, slirp->proxy_user,
Ditto.
> diff --git a/slirp/socks5.c b/slirp/socks5.c
> new file mode 100644
> index 000..2bba045
> --- /dev/null
> +++ b/slirp/socks5.c
> @@ -0,0 +1,371 @@
In v2 of the patch, this was said to have "some parts from nmap/ncat
GPLv2". Is that really not true any more? If any part of the file is
not original, it *has* to wear proper copyright notices, otherwise it's
copyright infrigement.
Also, see the bot build error report: doesn't exist on
windows,
#include
#include
#include
should be used instead.
Samuel
Re: [Qemu-devel] dns server not working in QEMU using usermode networking (SLIRP)
FONNEMANN Mark, on ven. 05 mai 2017 22:38:20 +, wrote:
> >Could you run tcpdump inside the guest so we are sure what the nslookup call
> >emits?
>
> Is there another way to determine this info? My guest OS is an embedded
> system with BusyBox 1.25 and not much else.
The attached patch should be dumping the same kind of information.
Samuel
diff --git a/slirp/ip_input.c b/slirp/ip_input.c
index 348e1dca5a..ada6f0d059 100644
--- a/slirp/ip_input.c
+++ b/slirp/ip_input.c
@@ -41,6 +41,7 @@
#include "qemu/osdep.h"
#include "slirp.h"
#include "ip_icmp.h"
+#include "qemu/log.h"
static struct ip *ip_reass(Slirp *slirp, struct ip *ip, struct ipq *fp);
static void ip_freef(Slirp *slirp, struct ipq *fp);
@@ -93,6 +94,8 @@ ip_input(struct mbuf *m)
ip = mtod(m, struct ip *);
+qemu_log("ip input %x -> %x %x\n", ntohl(ip->ip_src.s_addr),
ntohl(ip->ip_dst.s_addr), ip->ip_p);
+
if (ip->ip_v != IPVERSION) {
goto bad;
}
diff --git a/slirp/udp.c b/slirp/udp.c
index 227d779022..521c3a5a57 100644
--- a/slirp/udp.c
+++ b/slirp/udp.c
@@ -41,6 +41,7 @@
#include "qemu/osdep.h"
#include "slirp.h"
#include "ip_icmp.h"
+#include "qemu/log.h"
static uint8_t udp_tos(struct socket *so);
@@ -95,6 +96,7 @@ udp_input(register struct mbuf *m, int iphlen)
ip = mtod(m, struct ip *);
uh = (struct udphdr *)((caddr_t)ip + iphlen);
+qemu_log("udp input %u -> %u\n", (unsigned) ntohs(uh->uh_sport),
(unsigned) ntohs(uh->uh_dport));
/*
* Make mbuf data length reflect UDP length.
* If not enough data to reflect UDP length, drop.
Re: [Qemu-devel] [PATCH 1/1] slirp: don't zero ti_i since we acccess it later.
Hello, lepton, on mer. 03 mai 2017 11:35:05 -0700, wrote: > It sounds like a bug that caller set up a > right src and dst address and without set right ih_x1. I wouldn't bet on that. ih_x1 is only a filler, the caller can be using the structure only as a C structure, and it's only here just before the checksum computation that we really need ih_x1 to be 0. Samuel
Re: [Qemu-devel] dns server not working in QEMU using usermode networking (SLIRP)
FONNEMANN Mark, on dim. 30 avril 2017 12:42:41 +, wrote: > root@qemu:~# nslookup www.google.com >*** Unknown host > >nslookup: www.google.com: Unknown host Could you run tcpdump inside the guest so we are sure what the nslookup call emits? Samuel
Re: [Qemu-devel] dns server not working in QEMU using usermode networking (SLIRP)
FONNEMANN Mark, on sam. 29 avril 2017 23:15:08 +, wrote: > [mfonnemann@desktopPC qemu]$ /usr/local/bin/qemu-system-i386 -display curses > -readconfig NG.cfg --enable-kvm Could you show us your NG.cfg, your guest /sbin/ifconfig and /sbin/route output and /etc/resolv.conf content for that host? Samuel
Re: [Qemu-devel] dns server not working in QEMU using usermode networking (SLIRP)
FONNEMANN Mark, on sam. 29 avril 2017 20:06:09 +, wrote:
> [mfonnemann@localhost qemu]$ /usr/local/bin/qemu-system-i386 -display curses
> -readconfig qemu.cfg -D logfile
Ah, you are using curses... I've been putting logs onto stderr for
simplicity, not the proper qemu logs. Here is a fixed patch to get
output in the log.
Samuel
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -6,6 +6,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/log.h"
#include "qemu-common.h"
#include "slirp.h"
#include "ip_icmp.h"
@@ -814,11 +815,18 @@ void sotranslate_out(struct socket *so,
switch (addr->ss_family) {
case AF_INET:
+qemu_log("translating %x against %x %x\n",
+so->so_faddr.s_addr,
+slirp->vnetwork_mask.s_addr,
+slirp->vnameserver_addr.s_addr);
if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) ==
slirp->vnetwork_addr.s_addr) {
/* It's an alias */
+qemu_log("it's an alias\n");
if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
+qemu_log("it's DNS\n");
if (get_dns_addr(&sin->sin_addr) < 0) {
+qemu_log("didn't get DNS address");
sin->sin_addr = loopback_addr;
}
} else {
--- a/slirp/udp.c
+++ b/slirp/udp.c
@@ -39,6 +39,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/log.h"
#include "slirp.h"
#include "ip_icmp.h"
@@ -133,6 +134,10 @@ udp_input(register struct mbuf *m, int i
lhost4->sin_addr = ip->ip_src;
lhost4->sin_port = uh->uh_sport;
+if (ntohs(uh->uh_dport) == 53 &&
+ip->ip_dst.s_addr == slirp->vnameserver_addr.s_addr)
+qemu_log("UDP packet for DNS server\n");
+
/*
* handle DHCP/BOOTP
*/
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -22,6 +22,7 @@
* THE SOFTWARE.
*/
#include "qemu/osdep.h"
+#include "qemu/log.h"
#include "qemu-common.h"
#include "qemu/timer.h"
#include "qemu/error-report.h"
@@ -165,6 +166,7 @@ static int get_dns_addr_resolv_conf(int
#endif
while (fgets(buff, 512, f) != NULL) {
if (sscanf(buff, "nameserver%*[ \t]%256s", buff2) == 1) {
+qemu_log("got resolv.conf entry '%s'\n", buff2);
char *c = strchr(buff2, '%');
if (c) {
if_index = if_nametoindex(c + 1);
@@ -174,10 +176,16 @@ static int get_dns_addr_resolv_conf(int
}
if (!inet_pton(af, buff2, tmp_addr)) {
+qemu_log("couldn't parse it as %d\n", af);
continue;
}
/* If it's the first one, set it to dns_addr */
if (!found) {
+if (af == AF_INET)
+{
+struct in_addr *in = tmp_addr;
+qemu_log("parsed %x\n", in->s_addr);
+}
memcpy(pdns_addr, tmp_addr, addrlen);
memcpy(cached_addr, tmp_addr, addrlen);
if (scope_id) {
@@ -219,6 +227,7 @@ int get_dns_addr(struct in_addr *pdns_ad
if (dns_addr.s_addr != 0) {
int ret;
+qemu_log("dns_addr is cached: %x\n", dns_addr.s_addr);
ret = get_dns_addr_cached(pdns_addr, &dns_addr, sizeof(dns_addr),
&dns_addr_stat, &dns_addr_time);
if (ret <= 0) {
Re: [Qemu-devel] dns server not working in QEMU using usermode networking (SLIRP)
FONNEMANN Mark, on sam. 29 avril 2017 18:25:26 +, wrote:
> 0 packets received by filter
Ok, so nothing going out from qemu. Could you try to apply the attached
patch to qemu-2.9 and see what it makes qemu print when you resolve a
domain from the guest?
Samuel
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -814,11 +814,18 @@ void sotranslate_out(struct socket *so,
switch (addr->ss_family) {
case AF_INET:
+fprintf(stderr,"translating %x against %x %x\n",
+so->so_faddr.s_addr,
+slirp->vnetwork_mask.s_addr,
+slirp->vnameserver_addr.s_addr);
if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) ==
slirp->vnetwork_addr.s_addr) {
/* It's an alias */
+fprintf(stderr,"it's an alias\n");
if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
+fprintf(stderr,"it's DNS\n");
if (get_dns_addr(&sin->sin_addr) < 0) {
+fprintf(stderr,"didn't get DNS address");
sin->sin_addr = loopback_addr;
}
} else {
--- a/slirp/udp.c
+++ b/slirp/udp.c
@@ -133,6 +133,10 @@ udp_input(register struct mbuf *m, int i
lhost4->sin_addr = ip->ip_src;
lhost4->sin_port = uh->uh_sport;
+if (ntohs(uh->uh_dport) == 53 &&
+ip->ip_dst.s_addr == slirp->vnameserver_addr.s_addr)
+fprintf(stderr,"UDP packet for DNS server\n");
+
/*
* handle DHCP/BOOTP
*/
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -165,6 +165,7 @@ static int get_dns_addr_resolv_conf(int
#endif
while (fgets(buff, 512, f) != NULL) {
if (sscanf(buff, "nameserver%*[ \t]%256s", buff2) == 1) {
+fprintf(stderr,"got resolv.conf entry '%s'\n", buff2);
char *c = strchr(buff2, '%');
if (c) {
if_index = if_nametoindex(c + 1);
@@ -174,10 +175,16 @@ static int get_dns_addr_resolv_conf(int
}
if (!inet_pton(af, buff2, tmp_addr)) {
+fprintf(stderr,"couldn't parse it\n");
continue;
}
/* If it's the first one, set it to dns_addr */
if (!found) {
+if (af == AF_INET)
+{
+struct sockaddr_in *sin = tmp_addr;
+fprintf(stderr,"parsed %x\n", sin->sin_addr.s_addr);
+}
memcpy(pdns_addr, tmp_addr, addrlen);
memcpy(cached_addr, tmp_addr, addrlen);
if (scope_id) {
@@ -219,6 +226,7 @@ int get_dns_addr(struct in_addr *pdns_ad
if (dns_addr.s_addr != 0) {
int ret;
+fprintf(stderr,"dns_addr is cached: %x\n", dns_addr.s_addr);
ret = get_dns_addr_cached(pdns_addr, &dns_addr, sizeof(dns_addr),
&dns_addr_stat, &dns_addr_time);
if (ret <= 0) {
Re: [Qemu-devel] dns server not working in QEMU using usermode networking (SLIRP)
FONNEMANN Mark, on sam. 29 avril 2017 15:32:35 +, wrote: > >When running with a Linux host (since that's what I'm most familiar with), > >what do you have in /etc/resolv.conf? > > [mfonnemann@localhost ~]$ cat /etc/resolv.conf > # Generated by NetworkManager > nameserver 4.2.2.1 > nameserver 4.2.2.2 And I guess these do work fine on the host. Perhaps you should make sure that qemu does emit the DNS lookup request to these thanks to tcpdump -i any host 4.2.2.1 or host 4.2.2.2 while you run nslookup from the guest. Samuel
[Qemu-devel] [PULL 8/9] slirp: VMStatify socket level
From: "Dr. David Alan Gilbert"
Working up the stack, this replaces the slirp_socket_load/save
with VMState definitions.
A place holder for IPv6 support is added as a comment; it needs
testing once the rest of the IPv6 code is there.
Signed-off-by: Dr. David Alan Gilbert
Signed-off-by: Samuel Thibault
---
slirp/slirp.c | 219 +
slirp/socket.h | 6 +-
2 files changed, 145 insertions(+), 80 deletions(-)
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 8fc4e8df73..c3426648e5 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -1254,40 +1254,154 @@ static const VMStateDescription vmstate_slirp_sbuf = {
}
};
+static bool slirp_older_than_v4(void *opaque, int version_id)
+{
+return version_id < 4;
+}
-static void slirp_socket_save(QEMUFile *f, struct socket *so)
+static bool slirp_family_inet(void *opaque, int version_id)
{
-qemu_put_be32(f, so->so_urgc);
-qemu_put_be16(f, so->so_ffamily);
-switch (so->so_ffamily) {
-case AF_INET:
-qemu_put_be32(f, so->so_faddr.s_addr);
-qemu_put_be16(f, so->so_fport);
-break;
-default:
-error_report("so_ffamily unknown, unable to save so_faddr and"
- " so_fport");
+union slirp_sockaddr *ssa = (union slirp_sockaddr *)opaque;
+return ssa->ss.ss_family == AF_INET;
+}
+
+static int slirp_socket_pre_load(void *opaque)
+{
+struct socket *so = opaque;
+if (tcp_attach(so) < 0) {
+return -ENOMEM;
}
-qemu_put_be16(f, so->so_lfamily);
-switch (so->so_lfamily) {
-case AF_INET:
-qemu_put_be32(f, so->so_laddr.s_addr);
-qemu_put_be16(f, so->so_lport);
+/* Older versions don't load these fields */
+so->so_ffamily = AF_INET;
+so->so_lfamily = AF_INET;
+return 0;
+}
+
+#ifndef _WIN32
+#define VMSTATE_SIN4_ADDR(f, s, t) VMSTATE_UINT32_TEST(f, s, t)
+#else
+/* Win uses u_long rather than uint32_t - but it's still 32bits long */
+#define VMSTATE_SIN4_ADDR(f, s, t) VMSTATE_SINGLE_TEST(f, s, t, 0, \
+ vmstate_info_uint32, u_long)
+#endif
+
+/* The OS provided ss_family field isn't that portable; it's size
+ * and type varies (16/8 bit, signed, unsigned)
+ * and the values it contains aren't fully portable.
+ */
+typedef struct SS_FamilyTmpStruct {
+union slirp_sockaddr*parent;
+uint16_t portable_family;
+} SS_FamilyTmpStruct;
+
+#define SS_FAMILY_MIG_IPV4 2 /* Linux, BSD, Win... */
+#define SS_FAMILY_MIG_IPV6 10 /* Linux */
+#define SS_FAMILY_MIG_OTHER 0x
+
+static void ss_family_pre_save(void *opaque)
+{
+SS_FamilyTmpStruct *tss = opaque;
+
+tss->portable_family = SS_FAMILY_MIG_OTHER;
+
+if (tss->parent->ss.ss_family == AF_INET) {
+tss->portable_family = SS_FAMILY_MIG_IPV4;
+} else if (tss->parent->ss.ss_family == AF_INET6) {
+tss->portable_family = SS_FAMILY_MIG_IPV6;
+}
+}
+
+static int ss_family_post_load(void *opaque, int version_id)
+{
+SS_FamilyTmpStruct *tss = opaque;
+
+switch (tss->portable_family) {
+case SS_FAMILY_MIG_IPV4:
+tss->parent->ss.ss_family = AF_INET;
+break;
+case SS_FAMILY_MIG_IPV6:
+case 23: /* compatibility: AF_INET6 from mingw */
+case 28: /* compatibility: AF_INET6 from FreeBSD sys/socket.h */
+tss->parent->ss.ss_family = AF_INET6;
break;
default:
-error_report("so_ffamily unknown, unable to save so_laddr and"
- " so_lport");
+error_report("invalid ss_family type %x", tss->portable_family);
+return -EINVAL;
}
-qemu_put_byte(f, so->so_iptos);
-qemu_put_byte(f, so->so_emu);
-qemu_put_byte(f, so->so_type);
-qemu_put_be32(f, so->so_state);
-/* TODO: Build vmstate at this level */
-vmstate_save_state(f, &vmstate_slirp_sbuf, &so->so_rcv, 0);
-vmstate_save_state(f, &vmstate_slirp_sbuf, &so->so_snd, 0);
-vmstate_save_state(f, &vmstate_slirp_tcp, so->so_tcpcb, 0);
+
+return 0;
}
+static const VMStateDescription vmstate_slirp_ss_family = {
+.name = "slirp-socket-addr/ss_family",
+.pre_save = ss_family_pre_save,
+.post_load = ss_family_post_load,
+.fields = (VMStateField[]) {
+VMSTATE_UINT16(portable_family, SS_FamilyTmpStruct),
+VMSTATE_END_OF_LIST()
+}
+};
+
+static const VMStateDescription vmstate_slirp_socket_addr = {
+.name = "slirp-socket-addr",
+.version_id = 4,
+.fields = (VMStateField[]) {
+VMSTATE_WITH_TMP(union slirp_sockaddr, SS_FamilyTmpStruct,
+vmstate_slirp_ss_family),
+VMSTATE_SIN4_ADDR(sin.sin_addr.s_addr, union slirp_sockaddr,
+slirp_family_in
[Qemu-devel] [PULL 9/9] slirp: VMStatify remaining except for loop
From: "Dr. David Alan Gilbert"
This converts the remaining components, except for the top level
loop, to VMState.
Signed-off-by: Dr. David Alan Gilbert
Reviewed-by: Juan Quintela
Signed-off-by: Samuel Thibault
---
slirp/slirp.c | 48 +++-
1 file changed, 19 insertions(+), 29 deletions(-)
diff --git a/slirp/slirp.c b/slirp/slirp.c
index c3426648e5..2f2ec2c1b3 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -1402,15 +1402,25 @@ static const VMStateDescription vmstate_slirp_socket = {
}
};
-static void slirp_bootp_save(QEMUFile *f, Slirp *slirp)
-{
-int i;
+static const VMStateDescription vmstate_slirp_bootp_client = {
+.name = "slirp_bootpclient",
+.fields = (VMStateField[]) {
+VMSTATE_UINT16(allocated, BOOTPClient),
+VMSTATE_BUFFER(macaddr, BOOTPClient),
+VMSTATE_END_OF_LIST()
+}
+};
-for (i = 0; i < NB_BOOTP_CLIENTS; i++) {
-qemu_put_be16(f, slirp->bootp_clients[i].allocated);
-qemu_put_buffer(f, slirp->bootp_clients[i].macaddr, 6);
+static const VMStateDescription vmstate_slirp = {
+.name = "slirp",
+.version_id = 4,
+.fields = (VMStateField[]) {
+VMSTATE_UINT16_V(ip_id, Slirp, 2),
+VMSTATE_STRUCT_ARRAY(bootp_clients, Slirp, NB_BOOTP_CLIENTS, 3,
+ vmstate_slirp_bootp_client, BOOTPClient),
+VMSTATE_END_OF_LIST()
}
-}
+};
static void slirp_state_save(QEMUFile *f, void *opaque)
{
@@ -1430,22 +1440,10 @@ static void slirp_state_save(QEMUFile *f, void *opaque)
}
qemu_put_byte(f, 0);
-qemu_put_be16(f, slirp->ip_id);
-
-slirp_bootp_save(f, slirp);
+vmstate_save_state(f, &vmstate_slirp, slirp, NULL);
}
-static void slirp_bootp_load(QEMUFile *f, Slirp *slirp)
-{
-int i;
-
-for (i = 0; i < NB_BOOTP_CLIENTS; i++) {
-slirp->bootp_clients[i].allocated = qemu_get_be16(f);
-qemu_get_buffer(f, slirp->bootp_clients[i].macaddr, 6);
-}
-}
-
static int slirp_state_load(QEMUFile *f, void *opaque, int version_id)
{
Slirp *slirp = opaque;
@@ -1480,13 +1478,5 @@ static int slirp_state_load(QEMUFile *f, void *opaque,
int version_id)
so->extra = (void *)ex_ptr->ex_exec;
}
-if (version_id >= 2) {
-slirp->ip_id = qemu_get_be16(f);
-}
-
-if (version_id >= 3) {
-slirp_bootp_load(f, slirp);
-}
-
-return 0;
+return vmstate_load_state(f, &vmstate_slirp, slirp, version_id);
}
--
2.11.0
[Qemu-devel] [PULL 5/9] slirp: VMState conversion; tcpcb
From: "Dr. David Alan Gilbert"
Convert the migration of the struct tcpcb to use a VMStateDescription,
the rest of it will come later.
Mostly mechanical, except for conversion of some 'char' to uint8_t
to ensure portability.
Signed-off-by: Dr. David Alan Gilbert
Reviewed-by: Samuel Thibault
Reviewed-by: Juan Quintela
Signed-off-by: Samuel Thibault
---
slirp/slirp.c | 149
slirp/tcp_var.h | 6 +--
2 files changed, 57 insertions(+), 98 deletions(-)
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 9a50918346..2c2a589303 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -1133,53 +1133,62 @@ void slirp_socket_recv(Slirp *slirp, struct in_addr
guest_addr, int guest_port,
tcp_output(sototcpcb(so));
}
-static void slirp_tcp_save(QEMUFile *f, struct tcpcb *tp)
+static int slirp_tcp_post_load(void *opaque, int version)
{
-int i;
+tcp_template((struct tcpcb *)opaque);
-qemu_put_sbe16(f, tp->t_state);
-for (i = 0; i < TCPT_NTIMERS; i++)
-qemu_put_sbe16(f, tp->t_timer[i]);
-qemu_put_sbe16(f, tp->t_rxtshift);
-qemu_put_sbe16(f, tp->t_rxtcur);
-qemu_put_sbe16(f, tp->t_dupacks);
-qemu_put_be16(f, tp->t_maxseg);
-qemu_put_sbyte(f, tp->t_force);
-qemu_put_be16(f, tp->t_flags);
-qemu_put_be32(f, tp->snd_una);
-qemu_put_be32(f, tp->snd_nxt);
-qemu_put_be32(f, tp->snd_up);
-qemu_put_be32(f, tp->snd_wl1);
-qemu_put_be32(f, tp->snd_wl2);
-qemu_put_be32(f, tp->iss);
-qemu_put_be32(f, tp->snd_wnd);
-qemu_put_be32(f, tp->rcv_wnd);
-qemu_put_be32(f, tp->rcv_nxt);
-qemu_put_be32(f, tp->rcv_up);
-qemu_put_be32(f, tp->irs);
-qemu_put_be32(f, tp->rcv_adv);
-qemu_put_be32(f, tp->snd_max);
-qemu_put_be32(f, tp->snd_cwnd);
-qemu_put_be32(f, tp->snd_ssthresh);
-qemu_put_sbe16(f, tp->t_idle);
-qemu_put_sbe16(f, tp->t_rtt);
-qemu_put_be32(f, tp->t_rtseq);
-qemu_put_sbe16(f, tp->t_srtt);
-qemu_put_sbe16(f, tp->t_rttvar);
-qemu_put_be16(f, tp->t_rttmin);
-qemu_put_be32(f, tp->max_sndwnd);
-qemu_put_byte(f, tp->t_oobflags);
-qemu_put_byte(f, tp->t_iobc);
-qemu_put_sbe16(f, tp->t_softerror);
-qemu_put_byte(f, tp->snd_scale);
-qemu_put_byte(f, tp->rcv_scale);
-qemu_put_byte(f, tp->request_r_scale);
-qemu_put_byte(f, tp->requested_s_scale);
-qemu_put_be32(f, tp->ts_recent);
-qemu_put_be32(f, tp->ts_recent_age);
-qemu_put_be32(f, tp->last_ack_sent);
+return 0;
}
+static const VMStateDescription vmstate_slirp_tcp = {
+.name = "slirp-tcp",
+.version_id = 0,
+.post_load = slirp_tcp_post_load,
+.fields = (VMStateField[]) {
+VMSTATE_INT16(t_state, struct tcpcb),
+VMSTATE_INT16_ARRAY(t_timer, struct tcpcb, TCPT_NTIMERS),
+VMSTATE_INT16(t_rxtshift, struct tcpcb),
+VMSTATE_INT16(t_rxtcur, struct tcpcb),
+VMSTATE_INT16(t_dupacks, struct tcpcb),
+VMSTATE_UINT16(t_maxseg, struct tcpcb),
+VMSTATE_UINT8(t_force, struct tcpcb),
+VMSTATE_UINT16(t_flags, struct tcpcb),
+VMSTATE_UINT32(snd_una, struct tcpcb),
+VMSTATE_UINT32(snd_nxt, struct tcpcb),
+VMSTATE_UINT32(snd_up, struct tcpcb),
+VMSTATE_UINT32(snd_wl1, struct tcpcb),
+VMSTATE_UINT32(snd_wl2, struct tcpcb),
+VMSTATE_UINT32(iss, struct tcpcb),
+VMSTATE_UINT32(snd_wnd, struct tcpcb),
+VMSTATE_UINT32(rcv_wnd, struct tcpcb),
+VMSTATE_UINT32(rcv_nxt, struct tcpcb),
+VMSTATE_UINT32(rcv_up, struct tcpcb),
+VMSTATE_UINT32(irs, struct tcpcb),
+VMSTATE_UINT32(rcv_adv, struct tcpcb),
+VMSTATE_UINT32(snd_max, struct tcpcb),
+VMSTATE_UINT32(snd_cwnd, struct tcpcb),
+VMSTATE_UINT32(snd_ssthresh, struct tcpcb),
+VMSTATE_INT16(t_idle, struct tcpcb),
+VMSTATE_INT16(t_rtt, struct tcpcb),
+VMSTATE_UINT32(t_rtseq, struct tcpcb),
+VMSTATE_INT16(t_srtt, struct tcpcb),
+VMSTATE_INT16(t_rttvar, struct tcpcb),
+VMSTATE_UINT16(t_rttmin, struct tcpcb),
+VMSTATE_UINT32(max_sndwnd, struct tcpcb),
+VMSTATE_UINT8(t_oobflags, struct tcpcb),
+VMSTATE_UINT8(t_iobc, struct tcpcb),
+VMSTATE_INT16(t_softerror, struct tcpcb),
+VMSTATE_UINT8(snd_scale, struct tcpcb),
+VMSTATE_UINT8(rcv_scale, struct tcpcb),
+VMSTATE_UINT8(request_r_scale, struct tcpcb),
+VMSTATE_UINT8(requested_s_scale, struct tcpcb),
+VMSTATE_UINT32(ts_recent, struct tcpcb),
+VMSTATE_UINT32(ts_recent_age, struct tcpcb),
+VMSTATE_UINT32(last_ack_sent, struct tcpcb),
+VMSTATE_END_OF_LIST()
+}
+};
+
static void slirp_sbuf_save(QEMUFile *f, struct sbuf *sbuf)
{
uint32_t off;
@@ -1222,7 +1231,7 @@ static void slirp_so
[Qemu-devel] [PULL 6/9] slirp: VMStatify sbuf
From: "Dr. David Alan Gilbert"
Convert the sbuf structure to a VMStateDescription.
Note this uses the VMSTATE_WITH_TMP mechanism to calculate
and reload the offsets based on the pointers.
Signed-off-by: Dr. David Alan Gilbert
Reviewed-by: David Gibson
Reviewed-by: Juan Quintela
Signed-off-by: Samuel Thibault
---
slirp/sbuf.h | 4 +-
slirp/slirp.c | 116 ++
2 files changed, 78 insertions(+), 42 deletions(-)
diff --git a/slirp/sbuf.h b/slirp/sbuf.h
index efcec39a6b..a722ecb629 100644
--- a/slirp/sbuf.h
+++ b/slirp/sbuf.h
@@ -12,8 +12,8 @@
#define sbspace(sb) ((sb)->sb_datalen - (sb)->sb_cc)
struct sbuf {
- u_int sb_cc; /* actual chars in buffer */
- u_int sb_datalen; /* Length of data */
+ uint32_t sb_cc; /* actual chars in buffer */
+ uint32_t sb_datalen;/* Length of data */
char*sb_wptr; /* write pointer. points to where the next
* bytes should be written in the sbuf */
char*sb_rptr; /* read pointer. points to where the next
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 2c2a589303..8fc4e8df73 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -1189,19 +1189,72 @@ static const VMStateDescription vmstate_slirp_tcp = {
}
};
-static void slirp_sbuf_save(QEMUFile *f, struct sbuf *sbuf)
+/* The sbuf has a pair of pointers that are migrated as offsets;
+ * we calculate the offsets and restore the pointers using
+ * pre_save/post_load on a tmp structure.
+ */
+struct sbuf_tmp {
+struct sbuf *parent;
+uint32_t roff, woff;
+};
+
+static void sbuf_tmp_pre_save(void *opaque)
+{
+struct sbuf_tmp *tmp = opaque;
+tmp->woff = tmp->parent->sb_wptr - tmp->parent->sb_data;
+tmp->roff = tmp->parent->sb_rptr - tmp->parent->sb_data;
+}
+
+static int sbuf_tmp_post_load(void *opaque, int version)
{
-uint32_t off;
-
-qemu_put_be32(f, sbuf->sb_cc);
-qemu_put_be32(f, sbuf->sb_datalen);
-off = (uint32_t)(sbuf->sb_wptr - sbuf->sb_data);
-qemu_put_sbe32(f, off);
-off = (uint32_t)(sbuf->sb_rptr - sbuf->sb_data);
-qemu_put_sbe32(f, off);
-qemu_put_buffer(f, (unsigned char*)sbuf->sb_data, sbuf->sb_datalen);
+struct sbuf_tmp *tmp = opaque;
+uint32_t requested_len = tmp->parent->sb_datalen;
+
+/* Allocate the buffer space used by the field after the tmp */
+sbreserve(tmp->parent, tmp->parent->sb_datalen);
+
+if (tmp->parent->sb_datalen != requested_len) {
+return -ENOMEM;
+}
+if (tmp->woff >= requested_len ||
+tmp->roff >= requested_len) {
+error_report("invalid sbuf offsets r/w=%u/%u len=%u",
+ tmp->roff, tmp->woff, requested_len);
+return -EINVAL;
+}
+
+tmp->parent->sb_wptr = tmp->parent->sb_data + tmp->woff;
+tmp->parent->sb_rptr = tmp->parent->sb_data + tmp->roff;
+
+return 0;
}
+
+static const VMStateDescription vmstate_slirp_sbuf_tmp = {
+.name = "slirp-sbuf-tmp",
+.post_load = sbuf_tmp_post_load,
+.pre_save = sbuf_tmp_pre_save,
+.version_id = 0,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32(woff, struct sbuf_tmp),
+VMSTATE_UINT32(roff, struct sbuf_tmp),
+VMSTATE_END_OF_LIST()
+}
+};
+
+static const VMStateDescription vmstate_slirp_sbuf = {
+.name = "slirp-sbuf",
+.version_id = 0,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32(sb_cc, struct sbuf),
+VMSTATE_UINT32(sb_datalen, struct sbuf),
+VMSTATE_WITH_TMP(struct sbuf, struct sbuf_tmp, vmstate_slirp_sbuf_tmp),
+VMSTATE_VBUFFER_UINT32(sb_data, struct sbuf, 0, NULL, sb_datalen),
+VMSTATE_END_OF_LIST()
+}
+};
+
+
static void slirp_socket_save(QEMUFile *f, struct socket *so)
{
qemu_put_be32(f, so->so_urgc);
@@ -1229,8 +1282,9 @@ static void slirp_socket_save(QEMUFile *f, struct socket
*so)
qemu_put_byte(f, so->so_emu);
qemu_put_byte(f, so->so_type);
qemu_put_be32(f, so->so_state);
-slirp_sbuf_save(f, &so->so_rcv);
-slirp_sbuf_save(f, &so->so_snd);
+/* TODO: Build vmstate at this level */
+vmstate_save_state(f, &vmstate_slirp_sbuf, &so->so_rcv, 0);
+vmstate_save_state(f, &vmstate_slirp_sbuf, &so->so_snd, 0);
vmstate_save_state(f, &vmstate_slirp_tcp, so->so_tcpcb, 0);
}
@@ -1267,31 +1321,9 @@ static void slirp_state_save(QEMUFile *f, void *opaque)
slirp_bootp_save(f, slirp);
}
-static int slirp_sbuf_load(QEMUFile *f, struct sbuf *sbuf)
-{
-uint32_t off, sb_cc, sb_datalen;
-
-sb_cc = qemu_get_be32(f);
-sb_datalen = qemu_get_be32(f);
-
-sbreserve(sbuf, sb_datalen);
-
-if (sbuf->sb_datalen != sb_datalen)
-return -ENOMEM;
-
-
[Qemu-devel] [PULL 7/9] slirp: Common lhost/fhost union
From: "Dr. David Alan Gilbert"
The socket structure has a pair of unions for lhost and fhost
addresses; the unions are identical so split them out into
a separate union declaration.
Signed-off-by: Dr. David Alan Gilbert
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Juan Quintela
Signed-off-by: Samuel Thibault
---
slirp/socket.h | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/slirp/socket.h b/slirp/socket.h
index 8feed2aea4..c1be77eaf3 100644
--- a/slirp/socket.h
+++ b/slirp/socket.h
@@ -15,6 +15,12 @@
* Our socket structure
*/
+union slirp_sockaddr {
+struct sockaddr_storage ss;
+struct sockaddr_in sin;
+struct sockaddr_in6 sin6;
+};
+
struct socket {
struct socket *so_next,*so_prev; /* For a linked list of sockets */
@@ -31,22 +37,14 @@ struct socket {
struct tcpiphdr *so_ti; /* Pointer to the original ti within
* so_mconn, for non-blocking connections */
int so_urgc;
- union { /* foreign host */
- struct sockaddr_storage ss;
- struct sockaddr_in sin;
- struct sockaddr_in6 sin6;
- } fhost;
+ union slirp_sockaddr fhost; /* Foreign host */
#define so_faddr fhost.sin.sin_addr
#define so_fport fhost.sin.sin_port
#define so_faddr6 fhost.sin6.sin6_addr
#define so_fport6 fhost.sin6.sin6_port
#define so_ffamily fhost.ss.ss_family
- union { /* local host */
- struct sockaddr_storage ss;
- struct sockaddr_in sin;
- struct sockaddr_in6 sin6;
- } lhost;
+ union slirp_sockaddr lhost; /* Local host */
#define so_laddr lhost.sin.sin_addr
#define so_lport lhost.sin.sin_port
#define so_laddr6 lhost.sin6.sin6_addr
--
2.11.0
