qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length


From: Thomas Huth
Subject: Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length
Date: Mon, 11 Jan 2021 13:54:06 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0

On 11/01/2021 13.42, Miroslav Rezanina wrote:


----- Original Message -----
From: "Thomas Huth" <thuth@redhat.com>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>, mrezanin@redhat.com, 
qemu-devel@nongnu.org, "qemu-s390x"
<qemu-s390x@nongnu.org>
Sent: Monday, January 11, 2021 1:24:57 PM
Subject: Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length

On 11/01/2021 13.10, Philippe Mathieu-Daudé wrote:
Hi Miroslav,

On 1/11/21 12:30 PM, mrezanin@redhat.com wrote:
From: Miroslav Rezanina <mrezanin@redhat.com>

There are two cases when vm name is copied but closing \0 can be lost
in case name is too long (>=256 characters).

Updating length to copy so there is space for closing \0.

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
   target/s390x/kvm.c         | 2 +-
   target/s390x/misc_helper.c | 4 +++-
   2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index b8385e6b95..2313b5727e 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1918,7 +1918,7 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64
addr, uint8_t ar)
        */
       if (qemu_name) {
           strncpy((char *)sysib.ext_names[0], qemu_name,
-                sizeof(sysib.ext_names[0]));
+                sizeof(sysib.ext_names[0]) - 1);
       } else {
           strcpy((char *)sysib.ext_names[0], "KVMguest");
       }

What about using strpadcpy() instead?

Yes, strpadcpy is the better way here - this field has to be padded with
zeroes, so doing "- 1" is wrong here.

Hi Thomas,

as I wrote in reply to Phillipe - the array is memset to zeroes before the if 
so we
are sure it's padded with zeroes (in this occurrence, not true for second one).

Ok, but dropping the last character is still wrong here. The ext_names do not need to be terminated with a \0 if they have the full length.

 Thomas




reply via email to

[Prev in Thread] Current Thread [Next in Thread]