Skip to contentSkip to conversation

s390x/cpumodel: fix segmentation fault when baselining models

PATCH Superseded David Hildenbrand 3 participants 7 messages Jul 18, 2018

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 s390x/cpumodel: fix segmentation fault when baselining models

Usually, when baselining two CPU models, whereby one of them has base CPU features disabled (e.g. z14-base,msa=off), we fallback to an older model that did not have these features in the base model. We always try to create a "sane" CPU model (as far as possible), and one part of it is that removing base features is no good and to be avoided.

Now, if we disable base features that were part of a z900, we're out of luck. We won't find a CPU model and QEMU will segfault. This is a scenario that should never happen in real life, but it can be used to crash QEMU.

So let's make something like this:

{ "execute": "query-cpu-model-baseline",
"arguments" : { "modela": { "name": "z14-base", "props": {"esan3" : false}},

                  "modelb": { "name": "z14"}} }

Produce:

{"return": {"model": {"name": "z900-base", "props": {"esan3": false}}}}

Instead of segfaulting.

This could of course be improved (e.g. to z14-base,esan3=false), however as this ususally won't happen, let's just avoid crashes.

---
 target/s390x/cpu_models.c | 6 ++++++
 1 file changed, 6 insertions(+)
target/s390x/cpu_models.c +6 -0
index cfdbccf46d..13a5d4f095 100644
@@ -716,6 +716,12 @@ CpuModelBaselineInfo *arch_query_cpu_model_baseline(CpuModelInfo *infoa,
 
     model.def = s390_find_cpu_def(cpu_type, max_gen, max_gen_ga,
                                   model.features);
+
+    /* models without early base features (esan3) are bad - fallback to z900 */
+    if (!model.def) {
+        model.def = s390_find_cpu_def(0x2064, 7, 1, NULL);
+    }
+
     /* strip off features not part of the max model */
     bitmap_and(model.features, model.features, model.def->full_feat,
                S390_FEAT_MAX);
Signature · 1 line
2.17.1
#1 lore raw
Quoted 36 lines · David Hildenbrand

Usually, when baselining two CPU models, whereby one of them has base CPU features disabled (e.g. z14-base,msa=off), we fallback to an older model that did not have these features in the base model. We always try to create a "sane" CPU model (as far as possible), and one part of it is that removing base features is no good and to be avoided.

Now, if we disable base features that were part of a z900, we're out of luck. We won't find a CPU model and QEMU will segfault. This is a scenario that should never happen in real life, but it can be used to crash QEMU.

So let's make something like this:

{ "execute": "query-cpu-model-baseline",
"arguments" : { "modela": { "name": "z14-base", "props": {"esan3" : false}},

                  "modelb": { "name": "z14"}} }

Produce:

{"return": {"model": {"name": "z900-base", "props": {"esan3": false}}}}

Instead of segfaulting.

This could of course be improved (e.g. to z14-base,esan3=false), however as this ususally won't happen, let's just avoid crashes.

---
 target/s390x/cpu_models.c | 6 ++++++
 1 file changed, 6 insertions(+)
target/s390x/cpu_models.c +0 -0
index cfdbccf46d..13a5d4f095 100644

target/s390x/cpu_models.c CpuModelBaselineInfo *arch_query_cpu_model_baseline(CpuModelInfo *infoa, · line 724

Quoted 9 lines
target/s390x/cpu_models.c +6 -0
@@ -716,6 +716,12 @@ CpuModelBaselineInfo *arch_query_cpu_model_baseline(CpuModelInfo *infoa,
 
     model.def = s390_find_cpu_def(cpu_type, max_gen, max_gen_ga,
                                   model.features);
+
+    /* models without early base features (esan3) are bad - fallback to z900 */
+    if (!model.def) {
+        model.def = s390_find_cpu_def(0x2064, 7, 1, NULL);
+    }
+

Is there a way to not even return z900 but retuning an empty model (e.g. no model that matches) ?

Quoted 4 lines
target/s390x/cpu_models.c +0 -0
     /* strip off features not part of the max model */
     bitmap_and(model.features, model.features, model.def->full_feat,
                S390_FEAT_MAX);
#2 lore raw
Quoted 40 lines · Christian Borntraeger
Quoted 37 lines · David Hildenbrand

Usually, when baselining two CPU models, whereby one of them has base CPU features disabled (e.g. z14-base,msa=off), we fallback to an older model that did not have these features in the base model. We always try to create a "sane" CPU model (as far as possible), and one part of it is that removing base features is no good and to be avoided.

Now, if we disable base features that were part of a z900, we're out of luck. We won't find a CPU model and QEMU will segfault. This is a scenario that should never happen in real life, but it can be used to crash QEMU.

So let's make something like this:

{ "execute": "query-cpu-model-baseline",
"arguments" : { "modela": { "name": "z14-base", "props": {"esan3" : false}},

                  "modelb": { "name": "z14"}} }

Produce:

{"return": {"model": {"name": "z900-base", "props": {"esan3": false}}}}

Instead of segfaulting.

This could of course be improved (e.g. to z14-base,esan3=false), however as this ususally won't happen, let's just avoid crashes.

---
 target/s390x/cpu_models.c | 6 ++++++
 1 file changed, 6 insertions(+)
target/s390x/cpu_models.c +0 -0
index cfdbccf46d..13a5d4f095 100644
@@ -716,6 +716,12 @@ CpuModelBaselineInfo *arch_query_cpu_model_baseline(CpuModelInfo *infoa,
 
Quoted 11 lines

target/s390x/cpu_models.c CpuModelBaselineInfo *arch_query_cpu_model_baseline(CpuModelInfo *infoa, · line 724

Quoted 8 lines
target/s390x/cpu_models.c +6 -0
     model.def = s390_find_cpu_def(cpu_type, max_gen, max_gen_ga,
                                   model.features);
+
+    /* models without early base features (esan3) are bad - fallback to z900 */
+    if (!model.def) {
+        model.def = s390_find_cpu_def(0x2064, 7, 1, NULL);
+    }
+

Is there a way to not even return z900 but retuning an empty model (e.g. no model that matches) ?

An error would be an alternative.

Signature · 3 lines
Thanks,

David / dhildenb
#3 lore raw
Quoted 53 lines · David Hildenbrand
Quoted 40 lines · Christian Borntraeger
Quoted 37 lines · David Hildenbrand

Usually, when baselining two CPU models, whereby one of them has base CPU features disabled (e.g. z14-base,msa=off), we fallback to an older model that did not have these features in the base model. We always try to create a "sane" CPU model (as far as possible), and one part of it is that removing base features is no good and to be avoided.

Now, if we disable base features that were part of a z900, we're out of luck. We won't find a CPU model and QEMU will segfault. This is a scenario that should never happen in real life, but it can be used to crash QEMU.

So let's make something like this:

{ "execute": "query-cpu-model-baseline",
"arguments" : { "modela": { "name": "z14-base", "props": {"esan3" : false}},

                  "modelb": { "name": "z14"}} }

Produce:

{"return": {"model": {"name": "z900-base", "props": {"esan3": false}}}}

Instead of segfaulting.

This could of course be improved (e.g. to z14-base,esan3=false), however as this ususally won't happen, let's just avoid crashes.

---
 target/s390x/cpu_models.c | 6 ++++++
 1 file changed, 6 insertions(+)
target/s390x/cpu_models.c +0 -0
index cfdbccf46d..13a5d4f095 100644
@@ -716,6 +716,12 @@ CpuModelBaselineInfo *arch_query_cpu_model_baseline(CpuModelInfo *infoa,
 
Quoted 11 lines

target/s390x/cpu_models.c CpuModelBaselineInfo *arch_query_cpu_model_baseline(CpuModelInfo *infoa, · line 724

Quoted 8 lines
target/s390x/cpu_models.c +6 -0
     model.def = s390_find_cpu_def(cpu_type, max_gen, max_gen_ga,
                                   model.features);
+
+    /* models without early base features (esan3) are bad - fallback to z900 */
+    if (!model.def) {
+        model.def = s390_find_cpu_def(0x2064, 7, 1, NULL);
+    }
+

Is there a way to not even return z900 but retuning an empty model (e.g. no model that matches) ?

Quoted 1 line

An error would be an alternative.

As N3 means new in zarch (and backported to ESA390 mode), there is no machine without N3. So I would prefer an error.

#4 lore raw
Quoted 56 lines · Christian Borntraeger
Quoted 53 lines · David Hildenbrand
Quoted 51 lines · Christian Borntraeger
Quoted 36 lines · David Hildenbrand

Usually, when baselining two CPU models, whereby one of them has base CPU features disabled (e.g. z14-base,msa=off), we fallback to an older model that did not have these features in the base model. We always try to create a "sane" CPU model (as far as possible), and one part of it is that removing base features is no good and to be avoided.

Now, if we disable base features that were part of a z900, we're out of luck. We won't find a CPU model and QEMU will segfault. This is a scenario that should never happen in real life, but it can be used to crash QEMU.

So let's make something like this:

{ "execute": "query-cpu-model-baseline",
"arguments" : { "modela": { "name": "z14-base", "props": {"esan3" : false}},

                  "modelb": { "name": "z14"}} }

Produce:

{"return": {"model": {"name": "z900-base", "props": {"esan3": false}}}}

Instead of segfaulting.

This could of course be improved (e.g. to z14-base,esan3=false), however as this ususally won't happen, let's just avoid crashes.

---
 target/s390x/cpu_models.c | 6 ++++++
 1 file changed, 6 insertions(+)
target/s390x/cpu_models.c +0 -0
index cfdbccf46d..13a5d4f095 100644

target/s390x/cpu_models.c CpuModelBaselineInfo *arch_query_cpu_model_baseline(CpuModelInfo *infoa, · line 724

Quoted 9 lines
target/s390x/cpu_models.c +6 -0
@@ -716,6 +716,12 @@ CpuModelBaselineInfo *arch_query_cpu_model_baseline(CpuModelInfo *infoa,
 
     model.def = s390_find_cpu_def(cpu_type, max_gen, max_gen_ga,
                                   model.features);
+
+    /* models without early base features (esan3) are bad - fallback to z900 */
+    if (!model.def) {
+        model.def = s390_find_cpu_def(0x2064, 7, 1, NULL);
+    }
+

Is there a way to not even return z900 but retuning an empty model (e.g. no model that matches) ?

Quoted 5 lines
Quoted 1 line

An error would be an alternative.

As N3 means new in zarch (and backported to ESA390 mode), there is no machine without N3. So I would prefer an error.

target/s390x/cpu_models.c +0 -0
index cfdbccf46d..604898a882 100644
@@ -716,6 +716,14 @@ CpuModelBaselineInfo

*arch_query_cpu_model_baseline(CpuModelInfo *infoa,

     model.def = s390_find_cpu_def(cpu_type, max_gen, max_gen_ga,
                                   model.features);

+
+ /* models without early base features (esan3) are bad */
+ if (!model.def) {
+ error_setg(errp, "No compatible CPU model could be created as"
+ " important base features are disabled");
+ return NULL;
+ }
+

     /* strip off features not part of the max model */
     bitmap_and(model.features, model.features, model.def->full_feat,
                S390_FEAT_MAX);
Signature · 3 lines
Thanks,

David / dhildenb
#5 lore raw
Quoted 19 lines · On Wed, 18 Jul 2018 10:50:01 +0200 David Hildenbrand <[email protected]> wrote:
target/s390x/cpu_models.c +8 -0
index cfdbccf46d..604898a882 100644
@@ -716,6 +716,14 @@ CpuModelBaselineInfo
 *arch_query_cpu_model_baseline(CpuModelInfo *infoa,
 
     model.def = s390_find_cpu_def(cpu_type, max_gen, max_gen_ga,
                                   model.features);
+
+    /* models without early base features (esan3) are bad */
+    if (!model.def) {
+        error_setg(errp, "No compatible CPU model could be created as"
+                   " important base features are disabled");
+        return NULL;
+    }
+
     /* strip off features not part of the max model */
     bitmap_and(model.features, model.features, model.def->full_feat,
Quoted 3 lines
                S390_FEAT_MAX);

+1, would queue.

#6 lore raw
In reply to David Hildenbrand
Quoted 53 lines · On Wed, 18 Jul 2018 10:40:18 +0200 David Hildenbrand <[email protected]> wrote:
Quoted 40 lines · Christian Borntraeger
Quoted 37 lines · David Hildenbrand

Usually, when baselining two CPU models, whereby one of them has base CPU features disabled (e.g. z14-base,msa=off), we fallback to an older model that did not have these features in the base model. We always try to create a "sane" CPU model (as far as possible), and one part of it is that removing base features is no good and to be avoided.

Now, if we disable base features that were part of a z900, we're out of luck. We won't find a CPU model and QEMU will segfault. This is a scenario that should never happen in real life, but it can be used to crash QEMU.

So let's make something like this:

{ "execute": "query-cpu-model-baseline",
"arguments" : { "modela": { "name": "z14-base", "props": {"esan3" : false}},

                  "modelb": { "name": "z14"}} }

Produce:

{"return": {"model": {"name": "z900-base", "props": {"esan3": false}}}}

Instead of segfaulting.

This could of course be improved (e.g. to z14-base,esan3=false), however as this ususally won't happen, let's just avoid crashes.

---
 target/s390x/cpu_models.c | 6 ++++++
 1 file changed, 6 insertions(+)
target/s390x/cpu_models.c +0 -0
index cfdbccf46d..13a5d4f095 100644
@@ -716,6 +716,12 @@ CpuModelBaselineInfo *arch_query_cpu_model_baseline(CpuModelInfo *infoa,
 
Quoted 11 lines

target/s390x/cpu_models.c CpuModelBaselineInfo *arch_query_cpu_model_baseline(CpuModelInfo *infoa, · line 724

Quoted 8 lines
target/s390x/cpu_models.c +6 -0
     model.def = s390_find_cpu_def(cpu_type, max_gen, max_gen_ga,
                                   model.features);
+
+    /* models without early base features (esan3) are bad - fallback to z900 */
+    if (!model.def) {
+        model.def = s390_find_cpu_def(0x2064, 7, 1, NULL);
+    }
+  

Is there a way to not even return z900 but retuning an empty model (e.g. no model that matches) ?

Quoted 3 lines

An error would be an alternative.

An error looks a bit saner to me. As long as we avoid unexpected segfaults :)