[v3] config: Add function to check if fw_cfg exists

[SeaBIOS] [PATCH v3] config: Add function to check if fw_cfg exists

Posted by Petr Berky 8 years, 1 month ago

It was found qemu_get_present_cpus_count may return impossible
number of cpus because of not checking if fw_cfg exists before
using it. That may lead to undefined behavior of emulator,
in particular Bochs that freezes.

Signed-off-by: Petr Berky <petr.berky@email.cz>
---
 src/fw/paravirt.c | 12 +++++++++++-
 src/fw/paravirt.h |  1 +
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
index 707502d..5b23d78 100644
--- a/src/fw/paravirt.c
+++ b/src/fw/paravirt.c
@@ -32,9 +32,16 @@ u32 RamSize;
 u64 RamSizeOver4G;
 // Type of emulator platform.
 int PlatformRunningOn VARFSEG;
+// cfg enabled
+int cfg_enabled = 0;
 // cfg_dma enabled
 int cfg_dma_enabled = 0;
 
+inline int qemu_cfg_enabled(void)
+{
+    return cfg_enabled;
+}
+
 inline int qemu_cfg_dma_enabled(void)
 {
     return cfg_dma_enabled;
@@ -392,7 +399,9 @@ u16
 qemu_get_present_cpus_count(void)
 {
     u16 smp_count = 0;
-    qemu_cfg_read_entry(&smp_count, QEMU_CFG_NB_CPUS, sizeof(smp_count));
+    if (qemu_cfg_enabled()) {
+        qemu_cfg_read_entry(&smp_count, QEMU_CFG_NB_CPUS, sizeof(smp_count));
+    }
     u16 cmos_cpu_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1;
     if (smp_count < cmos_cpu_count) {
         smp_count = cmos_cpu_count;
@@ -571,6 +580,7 @@ void qemu_cfg_init(void)
             return;
 
     dprintf(1, "Found QEMU fw_cfg\n");
+    cfg_enabled = 1;
 
     // Detect DMA interface.
     u32 id;
diff --git a/src/fw/paravirt.h b/src/fw/paravirt.h
index 16f3d9a..a14d83e 100644
--- a/src/fw/paravirt.h
+++ b/src/fw/paravirt.h
@@ -49,6 +49,7 @@ static inline int runningOnKVM(void) {
 // QEMU_CFG_DMA ID bit
 #define QEMU_CFG_VERSION_DMA    2
 
+int qemu_cfg_enabled(void);
 int qemu_cfg_dma_enabled(void);
 void qemu_preinit(void);
 void qemu_platform_setup(void);
-- 
2.11.0


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v3] config: Add function to check if fw_cfg exists

Posted by Laszlo Ersek 8 years, 1 month ago

On 03/28/17 23:03, Petr Berky wrote:
> It was found qemu_get_present_cpus_count may return impossible
> number of cpus because of not checking if fw_cfg exists before
> using it. That may lead to undefined behavior of emulator,
> in particular Bochs that freezes.
> 
> Signed-off-by: Petr Berky <petr.berky@email.cz>
> ---
>  src/fw/paravirt.c | 12 +++++++++++-
>  src/fw/paravirt.h |  1 +
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> index 707502d..5b23d78 100644
> --- a/src/fw/paravirt.c
> +++ b/src/fw/paravirt.c
> @@ -32,9 +32,16 @@ u32 RamSize;
>  u64 RamSizeOver4G;
>  // Type of emulator platform.
>  int PlatformRunningOn VARFSEG;
> +// cfg enabled
> +int cfg_enabled = 0;
>  // cfg_dma enabled
>  int cfg_dma_enabled = 0;
>  
> +inline int qemu_cfg_enabled(void)
> +{
> +    return cfg_enabled;
> +}
> +
>  inline int qemu_cfg_dma_enabled(void)
>  {
>      return cfg_dma_enabled;
> @@ -392,7 +399,9 @@ u16
>  qemu_get_present_cpus_count(void)
>  {
>      u16 smp_count = 0;
> -    qemu_cfg_read_entry(&smp_count, QEMU_CFG_NB_CPUS, sizeof(smp_count));
> +    if (qemu_cfg_enabled()) {
> +        qemu_cfg_read_entry(&smp_count, QEMU_CFG_NB_CPUS, sizeof(smp_count));
> +    }
>      u16 cmos_cpu_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1;
>      if (smp_count < cmos_cpu_count) {
>          smp_count = cmos_cpu_count;
> @@ -571,6 +580,7 @@ void qemu_cfg_init(void)
>              return;
>  
>      dprintf(1, "Found QEMU fw_cfg\n");
> +    cfg_enabled = 1;
>  
>      // Detect DMA interface.
>      u32 id;
> diff --git a/src/fw/paravirt.h b/src/fw/paravirt.h
> index 16f3d9a..a14d83e 100644
> --- a/src/fw/paravirt.h
> +++ b/src/fw/paravirt.h
> @@ -49,6 +49,7 @@ static inline int runningOnKVM(void) {
>  // QEMU_CFG_DMA ID bit
>  #define QEMU_CFG_VERSION_DMA    2
>  
> +int qemu_cfg_enabled(void);
>  int qemu_cfg_dma_enabled(void);
>  void qemu_preinit(void);
>  void qemu_platform_setup(void);
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v3] config: Add function to check if fw_cfg exists

Posted by Kevin O'Connor 8 years, 1 month ago

On Wed, Mar 29, 2017 at 09:22:27AM +0200, Laszlo Ersek wrote:
> On 03/28/17 23:03, Petr Berky wrote:
> > It was found qemu_get_present_cpus_count may return impossible
> > number of cpus because of not checking if fw_cfg exists before
> > using it. That may lead to undefined behavior of emulator,
> > in particular Bochs that freezes.

Thanks - I applied this patch.

> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Unfortunately, I forgot to attach the reviewed-by before pushing.
Appreciate the review though.

-Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://www.coreboot.org/mailman/listinfo/seabios