[PATCH 2/2] qemu_process: Fix theoretical overflow in uint to bool typecast

Peter Krempa pkrempa at redhat.com
Wed Feb 9 09:08:11 UTC 2022


On Wed, Feb 09, 2022 at 09:39:50 +0100, Michal Privoznik wrote:
> The qemuPrepareNVRAM() function accepts three arguments and the
> last one being a boolean type. However, when the function is
> called from qemuProcessPrepareHost() the argument passed is a
> result of logical and of @flags (unsigned int) and
> VIR_QEMU_PROCESS_START_RESET_NVRAM value. In theory this is
> unsafe to do because if the value of the flag is ever changed

I don't think that this is accurate as casting a unsigned int (32bits)
to a bool (8 bits) works properly even when the lower 8 bits of the
value equal to 0:

$ cat bool.c
#include <stdint.h>
#include <stdio.h>
#include <limits.h>
#include <stdbool.h>

void p(char *s, unsigned long long v, uint8_t u, bool b) {
    printf("%30s: %30llu: %4u: %d\n", s, v, u, b);
}

#define E(val) \
    p(#val, (val), (val), (val))

#define T(type) \
    E(type - 1);\
    E(type);\
    E(type + 1)

int main() {
    printf("bool size: %u\n", sizeof(bool));
    E(0);
    T(CHAR_MAX);
    T(UCHAR_MAX);
    T(UINT8_MAX);
    T(INT_MAX);
    T(UINT_MAX);
}


$ ./a.out
bool size: 1
                             0:                              0:    0: 0
                      0x7f - 1:                            126:  126: 1
                          0x7f:                            127:  127: 1
                      0x7f + 1:                            128:  128: 1
            (0x7f * 2 + 1) - 1:                            254:  254: 1
                (0x7f * 2 + 1):                            255:  255: 1
            (0x7f * 2 + 1) + 1:                            256:    0: 1
                     (255) - 1:                            254:  254: 1
                         (255):                            255:  255: 1
                     (255) + 1:                            256:    0: 1
                0x7fffffff - 1:                     2147483646:  254: 1
                    0x7fffffff:                     2147483647:  255: 1
                0x7fffffff + 1:           18446744071562067968:    0: 1
    (0x7fffffff * 2U + 1U) - 1:                     4294967294:  254: 1
        (0x7fffffff * 2U + 1U):                     4294967295:  255: 1
    (0x7fffffff * 2U + 1U) + 1:                              0:    0: 0



> then this expression might overflow. Do what we do elsewhere:
> double negation.

I have no problem with the code change to make it use our common
pattern.

> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_process.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 7066696f31..24873f6fb7 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -6983,7 +6983,7 @@ qemuProcessPrepareHost(virQEMUDriver *driver,
>          qemuProcessMakeDir(driver, vm, priv->channelTargetDir) < 0)
>          return -1;
>  
> -    if (qemuPrepareNVRAM(driver, vm, flags & VIR_QEMU_PROCESS_START_RESET_NVRAM) < 0)
> +    if (qemuPrepareNVRAM(driver, vm, !!(flags & VIR_QEMU_PROCESS_START_RESET_NVRAM)) < 0)
>          return -1;
>  
>      if (vm->def->vsock) {
> -- 
> 2.34.1
> 




More information about the libvir-list mailing list