Message ID | 20191104173654.30125-6-alex.bennee@linaro.org |
---|---|
State | New |
Headers | show |
Series | testing/next (netbsd stuff) | expand |
On Mon, Nov 04, 2019 at 05:36:53PM +0000, Alex Bennée wrote: > This test has been unstable on NetBSD for awhile. It seems the > mechanism used to listen to a random port is a Linux-ism (although a > received wisdom Linux-ism rather than a well documented one). As Hmm. I got reports of this issue and tried to reproduce, but I was never able to. I thought I had fixed it via other means, but I had no idea this was the cause of the issue. It's not a Linux-ism, I don't think, Richard Steven's "Unix Network Programming" mentions that is how this works (at least on UDP), and that is pre-Linux. That's probably where I got this technique. I've saw some web pages mention that Solaris and Windows do it this way. However, it's not specified, so it's probably a bad idea. The only way I can think to do it another way is to remove the bind() call, then it should randomly assign the port (per the spec, I think). The trouble with that is the address will be INADDR_ANY, so it will be bound on other interfaces besides the loopback, which make me slightly worried from a security point of view. I'm ok with this being linux-only, but I'd like to fix it so it works everywhere. -corey > working around would add more hard to test complexity to the test I've > gone for the easier option of making it CONFIG_LINUX only. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Cc: Corey Minyard <cminyard@mvista.com> > Cc: Kamil Rytarowski <kamil@netbsd.org> > --- > tests/Makefile.include | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tests/Makefile.include b/tests/Makefile.include > index 534ee487436..8566f5f119d 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -177,7 +177,9 @@ check-qtest-i386-$(CONFIG_SGA) += tests/boot-serial-test$(EXESUF) > check-qtest-i386-$(CONFIG_SLIRP) += tests/pxe-test$(EXESUF) > check-qtest-i386-y += tests/rtc-test$(EXESUF) > check-qtest-i386-$(CONFIG_ISA_IPMI_KCS) += tests/ipmi-kcs-test$(EXESUF) > +ifdef CONFIG_LINUX > check-qtest-i386-$(CONFIG_ISA_IPMI_BT) += tests/ipmi-bt-test$(EXESUF) > +endif > check-qtest-i386-y += tests/i440fx-test$(EXESUF) > check-qtest-i386-y += tests/fw_cfg-test$(EXESUF) > check-qtest-i386-y += tests/device-plug-test$(EXESUF) > -- > 2.20.1 >
Corey Minyard <cminyard@mvista.com> writes: > On Mon, Nov 04, 2019 at 05:36:53PM +0000, Alex Bennée wrote: >> This test has been unstable on NetBSD for awhile. It seems the >> mechanism used to listen to a random port is a Linux-ism (although a >> received wisdom Linux-ism rather than a well documented one). As > > Hmm. I got reports of this issue and tried to reproduce, but I was > never able to. I thought I had fixed it via other means, but I had no > idea this was the cause of the issue. > > It's not a Linux-ism, I don't think, Richard Steven's "Unix Network > Programming" mentions that is how this works (at least on UDP), and that > is pre-Linux. That's probably where I got this technique. I've saw > some web pages mention that Solaris and Windows do it this way. > > However, it's not specified, so it's probably a bad idea. The only way > I can think to do it another way is to remove the bind() call, then it > should randomly assign the port (per the spec, I think). The trouble > with that is the address will be INADDR_ANY, so it will be bound on > other interfaces besides the loopback, which make me slightly worried > from a security point of view. The other option is to keep the bind but seed it ourselves with a random port. However then you need to loop until you succeed and avoid locking up if something fatal has gone wrong. > I'm ok with this being linux-only, but I'd like to fix it so it works > everywhere. One option could be to use the glib networking stuff to set this up. That has options for creating dynamic ports for incoming connections although the docs are a bit light on details. g_socket_listener_add_any_inet_port certainly seems to do the retry approach (for a weirdly arbitrary 37 times). It leaves the fancy stuff to a hopefully well tested library although I note QEMU's own use of g_io APIs seems to be pretty sporadic. > > -corey > >> working around would add more hard to test complexity to the test I've >> gone for the easier option of making it CONFIG_LINUX only. >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> Cc: Corey Minyard <cminyard@mvista.com> >> Cc: Kamil Rytarowski <kamil@netbsd.org> >> --- >> tests/Makefile.include | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/tests/Makefile.include b/tests/Makefile.include >> index 534ee487436..8566f5f119d 100644 >> --- a/tests/Makefile.include >> +++ b/tests/Makefile.include >> @@ -177,7 +177,9 @@ check-qtest-i386-$(CONFIG_SGA) += tests/boot-serial-test$(EXESUF) >> check-qtest-i386-$(CONFIG_SLIRP) += tests/pxe-test$(EXESUF) >> check-qtest-i386-y += tests/rtc-test$(EXESUF) >> check-qtest-i386-$(CONFIG_ISA_IPMI_KCS) += tests/ipmi-kcs-test$(EXESUF) >> +ifdef CONFIG_LINUX >> check-qtest-i386-$(CONFIG_ISA_IPMI_BT) += tests/ipmi-bt-test$(EXESUF) >> +endif >> check-qtest-i386-y += tests/i440fx-test$(EXESUF) >> check-qtest-i386-y += tests/fw_cfg-test$(EXESUF) >> check-qtest-i386-y += tests/device-plug-test$(EXESUF) >> -- >> 2.20.1 >> -- Alex Bennée
diff --git a/tests/Makefile.include b/tests/Makefile.include index 534ee487436..8566f5f119d 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -177,7 +177,9 @@ check-qtest-i386-$(CONFIG_SGA) += tests/boot-serial-test$(EXESUF) check-qtest-i386-$(CONFIG_SLIRP) += tests/pxe-test$(EXESUF) check-qtest-i386-y += tests/rtc-test$(EXESUF) check-qtest-i386-$(CONFIG_ISA_IPMI_KCS) += tests/ipmi-kcs-test$(EXESUF) +ifdef CONFIG_LINUX check-qtest-i386-$(CONFIG_ISA_IPMI_BT) += tests/ipmi-bt-test$(EXESUF) +endif check-qtest-i386-y += tests/i440fx-test$(EXESUF) check-qtest-i386-y += tests/fw_cfg-test$(EXESUF) check-qtest-i386-y += tests/device-plug-test$(EXESUF)
This test has been unstable on NetBSD for awhile. It seems the mechanism used to listen to a random port is a Linux-ism (although a received wisdom Linux-ism rather than a well documented one). As working around would add more hard to test complexity to the test I've gone for the easier option of making it CONFIG_LINUX only. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: Corey Minyard <cminyard@mvista.com> Cc: Kamil Rytarowski <kamil@netbsd.org> --- tests/Makefile.include | 2 ++ 1 file changed, 2 insertions(+)