Message ID | 20190418034501.5038-1-ehabkost@redhat.com |
---|---|
State | New |
Headers | show |
Series | cpu: Fix crash with empty -cpu option | expand |
On Thu, Apr 18, 2019 at 12:45:01AM -0300, Eduardo Habkost wrote: > Fix the following crash: > > $ qemu-system-x86_64 -cpu '' > qemu-system-x86_64: qom/cpu.c:291: cpu_class_by_name: \ > Assertion `cpu_model && cc->class_by_name' failed. > > Regression test script included. > > Fixes: commit 99193d8f2ef5 ("cpu: drop unnecessary NULL check and cpu_common_class_by_name()") > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > exec.c | 4 ++++ > tests/acceptance/empty_cpu_model.py | 19 +++++++++++++++++++ > 2 files changed, 23 insertions(+) > create mode 100644 tests/acceptance/empty_cpu_model.py > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Tested-by: Stefano Garzarella <sgarzare@redhat.com> Thanks, Stefano
On Thu, 18 Apr 2019 00:45:01 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > Fix the following crash: > > $ qemu-system-x86_64 -cpu '' > qemu-system-x86_64: qom/cpu.c:291: cpu_class_by_name: \ > Assertion `cpu_model && cc->class_by_name' failed. > > Regression test script included. > > Fixes: commit 99193d8f2ef5 ("cpu: drop unnecessary NULL check and cpu_common_class_by_name()") > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > exec.c | 4 ++++ > tests/acceptance/empty_cpu_model.py | 19 +++++++++++++++++++ > 2 files changed, 23 insertions(+) > create mode 100644 tests/acceptance/empty_cpu_model.py > > diff --git a/exec.c b/exec.c > index 1ca95df9d8..d816b38863 100644 > --- a/exec.c > +++ b/exec.c > @@ -999,6 +999,10 @@ const char *parse_cpu_option(MachineState *machine, const char *cpu_option) > const char *cpu_type; > > model_pieces = g_strsplit(cpu_option, ",", 2); > + if (!model_pieces[0]) { > + error_report("-cpu option cannot be empty"); > + exit(1); s/1/EXIT_FAILURE/ > + } > > cc = lookup_cpu_class(model_pieces[0], &error_fatal); > cpu_type = object_class_get_name(OBJECT_CLASS(cc)); > diff --git a/tests/acceptance/empty_cpu_model.py b/tests/acceptance/empty_cpu_model.py > new file mode 100644 > index 0000000000..3f4f663582 > --- /dev/null > +++ b/tests/acceptance/empty_cpu_model.py > @@ -0,0 +1,19 @@ > +# Check for crash when using empty -cpu option > +# > +# Copyright (c) 2019 Red Hat, Inc. > +# > +# Author: > +# Eduardo Habkost <ehabkost@redhat.com> > +# > +# This work is licensed under the terms of the GNU GPL, version 2 or > +# later. See the COPYING file in the top-level directory. > +import subprocess > +from avocado_qemu import Test > + > +class EmptyCPUModel(Test): > + def test(self): > + cmd = [self.qemu_bin, '-S', '-display', 'none', '-machine', 'none', '-cpu', ''] > + r = subprocess.run(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE) > + self.assertEquals(r.returncode, 1, "QEMU exit code should be 1") > + self.assertEquals(r.stdout, b'', "QEMU stdout should be empty") > + self.assertNotEquals(r.stderr, b'', "QEMU stderr shouldn't be empty")
On Thu, Apr 18, 2019 at 01:22:16PM +0200, Igor Mammedov wrote: > On Thu, 18 Apr 2019 00:45:01 -0300 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > Fix the following crash: > > > > $ qemu-system-x86_64 -cpu '' > > qemu-system-x86_64: qom/cpu.c:291: cpu_class_by_name: \ > > Assertion `cpu_model && cc->class_by_name' failed. > > > > Regression test script included. > > > > Fixes: commit 99193d8f2ef5 ("cpu: drop unnecessary NULL check and cpu_common_class_by_name()") > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > exec.c | 4 ++++ > > tests/acceptance/empty_cpu_model.py | 19 +++++++++++++++++++ > > 2 files changed, 23 insertions(+) > > create mode 100644 tests/acceptance/empty_cpu_model.py > > > > diff --git a/exec.c b/exec.c > > index 1ca95df9d8..d816b38863 100644 > > --- a/exec.c > > +++ b/exec.c > > @@ -999,6 +999,10 @@ const char *parse_cpu_option(MachineState *machine, const char *cpu_option) > > const char *cpu_type; > > > > model_pieces = g_strsplit(cpu_option, ",", 2); > > + if (!model_pieces[0]) { > > + error_report("-cpu option cannot be empty"); > > + exit(1); > > s/1/EXIT_FAILURE/ Why? There are 753 instances of "exit(1)" in the tree, and 171 instances of "exit(EXIT_FAILURE)".
On Thu, 18 Apr 2019 10:23:29 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Thu, Apr 18, 2019 at 01:22:16PM +0200, Igor Mammedov wrote: > > On Thu, 18 Apr 2019 00:45:01 -0300 > > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > > > Fix the following crash: > > > > > > $ qemu-system-x86_64 -cpu '' > > > qemu-system-x86_64: qom/cpu.c:291: cpu_class_by_name: \ > > > Assertion `cpu_model && cc->class_by_name' failed. > > > > > > Regression test script included. > > > > > > Fixes: commit 99193d8f2ef5 ("cpu: drop unnecessary NULL check and cpu_common_class_by_name()") > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > > --- > > > exec.c | 4 ++++ > > > tests/acceptance/empty_cpu_model.py | 19 +++++++++++++++++++ > > > 2 files changed, 23 insertions(+) > > > create mode 100644 tests/acceptance/empty_cpu_model.py > > > > > > diff --git a/exec.c b/exec.c > > > index 1ca95df9d8..d816b38863 100644 > > > --- a/exec.c > > > +++ b/exec.c > > > @@ -999,6 +999,10 @@ const char *parse_cpu_option(MachineState *machine, const char *cpu_option) > > > const char *cpu_type; > > > > > > model_pieces = g_strsplit(cpu_option, ",", 2); > > > + if (!model_pieces[0]) { > > > + error_report("-cpu option cannot be empty"); > > > + exit(1); > > > > s/1/EXIT_FAILURE/ > > Why? it's more descriptive in general > There are 753 instances of "exit(1)" in the tree, and 171 > instances of "exit(EXIT_FAILURE)". Bad example in past is probably not a good justification, especially when adding new code (but I'm not going to argue about it).
On Thu, Apr 18, 2019 at 12:45:01AM -0300, Eduardo Habkost wrote: > Fix the following crash: > > $ qemu-system-x86_64 -cpu '' > qemu-system-x86_64: qom/cpu.c:291: cpu_class_by_name: \ > Assertion `cpu_model && cc->class_by_name' failed. > > Regression test script included. > > Fixes: commit 99193d8f2ef5 ("cpu: drop unnecessary NULL check and cpu_common_class_by_name()") > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Queued on machine-next.
diff --git a/exec.c b/exec.c index 1ca95df9d8..d816b38863 100644 --- a/exec.c +++ b/exec.c @@ -999,6 +999,10 @@ const char *parse_cpu_option(MachineState *machine, const char *cpu_option) const char *cpu_type; model_pieces = g_strsplit(cpu_option, ",", 2); + if (!model_pieces[0]) { + error_report("-cpu option cannot be empty"); + exit(1); + } cc = lookup_cpu_class(model_pieces[0], &error_fatal); cpu_type = object_class_get_name(OBJECT_CLASS(cc)); diff --git a/tests/acceptance/empty_cpu_model.py b/tests/acceptance/empty_cpu_model.py new file mode 100644 index 0000000000..3f4f663582 --- /dev/null +++ b/tests/acceptance/empty_cpu_model.py @@ -0,0 +1,19 @@ +# Check for crash when using empty -cpu option +# +# Copyright (c) 2019 Red Hat, Inc. +# +# Author: +# Eduardo Habkost <ehabkost@redhat.com> +# +# This work is licensed under the terms of the GNU GPL, version 2 or +# later. See the COPYING file in the top-level directory. +import subprocess +from avocado_qemu import Test + +class EmptyCPUModel(Test): + def test(self): + cmd = [self.qemu_bin, '-S', '-display', 'none', '-machine', 'none', '-cpu', ''] + r = subprocess.run(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE) + self.assertEquals(r.returncode, 1, "QEMU exit code should be 1") + self.assertEquals(r.stdout, b'', "QEMU stdout should be empty") + self.assertNotEquals(r.stderr, b'', "QEMU stderr shouldn't be empty")
Fix the following crash: $ qemu-system-x86_64 -cpu '' qemu-system-x86_64: qom/cpu.c:291: cpu_class_by_name: \ Assertion `cpu_model && cc->class_by_name' failed. Regression test script included. Fixes: commit 99193d8f2ef5 ("cpu: drop unnecessary NULL check and cpu_common_class_by_name()") Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- exec.c | 4 ++++ tests/acceptance/empty_cpu_model.py | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 tests/acceptance/empty_cpu_model.py