diff mbox series

[08/12] pci: Fix silent truncation of pcie_aer_inject_error argument

Message ID 20221128080202.2570543-9-armbru@redhat.com (mailing list archive)
State New, archived
Headers show
Series pci: Move and clean up monitor command code | expand

Commit Message

Markus Armbruster Nov. 28, 2022, 8:01 a.m. UTC
PCI AER error status is 32 bit.  When the HMP command's second
argument parses as a number, values greater than ULONG_MAX get
rejected, but values between UINT32_MAX+1 and ULONG_MAX get silently
truncated.  Fix to reject them, too.

While there, use qemu_strtoul() instead of strtoul() so checkpatch.pl
won't complain.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pci/pcie_aer.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Dr. David Alan Gilbert Nov. 29, 2022, 12:14 p.m. UTC | #1
* Markus Armbruster (armbru@redhat.com) wrote:
> PCI AER error status is 32 bit.  When the HMP command's second
> argument parses as a number, values greater than ULONG_MAX get
> rejected, but values between UINT32_MAX+1 and ULONG_MAX get silently
> truncated.  Fix to reject them, too.
> 
> While there, use qemu_strtoul() instead of strtoul() so checkpatch.pl
> won't complain.

WOuldn't qemu_strtoui do the num > UINT32_MAX for you?

Dave

> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/pci/pcie_aer.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
> index eff62f3945..ccca5a81cc 100644
> --- a/hw/pci/pcie_aer.c
> +++ b/hw/pci/pcie_aer.c
> @@ -30,6 +30,7 @@
>  #include "hw/pci/pci_bus.h"
>  #include "hw/pci/pcie_regs.h"
>  #include "qapi/error.h"
> +#include "qemu/cutils.h"
>  
>  //#define DEBUG_PCIE
>  #ifdef DEBUG_PCIE
> @@ -963,6 +964,7 @@ static int do_pcie_aer_inject_error(Monitor *mon,
>      const char *id = qdict_get_str(qdict, "id");
>      const char *error_name;
>      uint32_t error_status;
> +    unsigned long num;
>      bool correctable;
>      PCIDevice *dev;
>      PCIEAERErr err;
> @@ -983,14 +985,14 @@ static int do_pcie_aer_inject_error(Monitor *mon,
>  
>      error_name = qdict_get_str(qdict, "error_status");
>      if (pcie_aer_parse_error_string(error_name, &error_status, &correctable)) {
> -        char *e = NULL;
> -        error_status = strtoul(error_name, &e, 0);
> -        correctable = qdict_get_try_bool(qdict, "correctable", false);
> -        if (!e || *e != '\0') {
> +        if (qemu_strtoul(error_name, NULL, 0, &num) < 0
> +            || num > UINT32_MAX) {
>              monitor_printf(mon, "invalid error status value. \"%s\"",
>                             error_name);
>              return -EINVAL;
>          }
> +        error_status = num;
> +        correctable = qdict_get_try_bool(qdict, "correctable", false);
>      }
>      err.status = error_status;
>      err.source_id = pci_requester_id(dev);
> -- 
> 2.37.3
>
Markus Armbruster Nov. 30, 2022, 6:40 p.m. UTC | #2
"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Markus Armbruster (armbru@redhat.com) wrote:
>> PCI AER error status is 32 bit.  When the HMP command's second
>> argument parses as a number, values greater than ULONG_MAX get
>> rejected, but values between UINT32_MAX+1 and ULONG_MAX get silently
>> truncated.  Fix to reject them, too.
>> 
>> While there, use qemu_strtoul() instead of strtoul() so checkpatch.pl
>> won't complain.
>
> WOuldn't qemu_strtoui do the num > UINT32_MAX for you?

Yes, that's better.

> Dave
>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  hw/pci/pcie_aer.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>> 
>> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
>> index eff62f3945..ccca5a81cc 100644
>> --- a/hw/pci/pcie_aer.c
>> +++ b/hw/pci/pcie_aer.c
>> @@ -30,6 +30,7 @@
>>  #include "hw/pci/pci_bus.h"
>>  #include "hw/pci/pcie_regs.h"
>>  #include "qapi/error.h"
>> +#include "qemu/cutils.h"
>>  
>>  //#define DEBUG_PCIE
>>  #ifdef DEBUG_PCIE
>> @@ -963,6 +964,7 @@ static int do_pcie_aer_inject_error(Monitor *mon,
>>      const char *id = qdict_get_str(qdict, "id");
>>      const char *error_name;
>>      uint32_t error_status;
>> +    unsigned long num;
>>      bool correctable;
>>      PCIDevice *dev;
>>      PCIEAERErr err;
>> @@ -983,14 +985,14 @@ static int do_pcie_aer_inject_error(Monitor *mon,
>>  
>>      error_name = qdict_get_str(qdict, "error_status");
>>      if (pcie_aer_parse_error_string(error_name, &error_status, &correctable)) {
>> -        char *e = NULL;
>> -        error_status = strtoul(error_name, &e, 0);
>> -        correctable = qdict_get_try_bool(qdict, "correctable", false);
>> -        if (!e || *e != '\0') {
>> +        if (qemu_strtoul(error_name, NULL, 0, &num) < 0
>> +            || num > UINT32_MAX) {
>>              monitor_printf(mon, "invalid error status value. \"%s\"",
>>                             error_name);
>>              return -EINVAL;
>>          }
>> +        error_status = num;
>> +        correctable = qdict_get_try_bool(qdict, "correctable", false);
>>      }
>>      err.status = error_status;
>>      err.source_id = pci_requester_id(dev);
>> -- 
>> 2.37.3
>>
diff mbox series

Patch

diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index eff62f3945..ccca5a81cc 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -30,6 +30,7 @@ 
 #include "hw/pci/pci_bus.h"
 #include "hw/pci/pcie_regs.h"
 #include "qapi/error.h"
+#include "qemu/cutils.h"
 
 //#define DEBUG_PCIE
 #ifdef DEBUG_PCIE
@@ -963,6 +964,7 @@  static int do_pcie_aer_inject_error(Monitor *mon,
     const char *id = qdict_get_str(qdict, "id");
     const char *error_name;
     uint32_t error_status;
+    unsigned long num;
     bool correctable;
     PCIDevice *dev;
     PCIEAERErr err;
@@ -983,14 +985,14 @@  static int do_pcie_aer_inject_error(Monitor *mon,
 
     error_name = qdict_get_str(qdict, "error_status");
     if (pcie_aer_parse_error_string(error_name, &error_status, &correctable)) {
-        char *e = NULL;
-        error_status = strtoul(error_name, &e, 0);
-        correctable = qdict_get_try_bool(qdict, "correctable", false);
-        if (!e || *e != '\0') {
+        if (qemu_strtoul(error_name, NULL, 0, &num) < 0
+            || num > UINT32_MAX) {
             monitor_printf(mon, "invalid error status value. \"%s\"",
                            error_name);
             return -EINVAL;
         }
+        error_status = num;
+        correctable = qdict_get_try_bool(qdict, "correctable", false);
     }
     err.status = error_status;
     err.source_id = pci_requester_id(dev);