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

[Philippe Mathieu-Daudé]

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?

    strpadcpy((char *)sysib.sysib_322.ext_names[0],
              sizeof(sysib.sysib_322.ext_names[0]),
              qemu_name ?: "KVMguest", '\0');

> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> index 58dbc023eb..7c478b9e58 100644
> --- a/target/s390x/misc_helper.c
> +++ b/target/s390x/misc_helper.c
> @@ -369,8 +369,10 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0, 
> uint64_t r0, uint64_t r1)
>                  ebcdic_put(sysib.sysib_322.vm[0].name, qemu_name,
>                             MIN(sizeof(sysib.sysib_322.vm[0].name),
>                                 strlen(qemu_name)));
> +             memset((char *)sysib.sysib_322.ext_names[0], 0, 
> +                    sizeof(sysib.sysib_322.ext_names[0]));
>                  strncpy((char *)sysib.sysib_322.ext_names[0], qemu_name,
> -                        sizeof(sysib.sysib_322.ext_names[0]));
> +                        sizeof(sysib.sysib_322.ext_names[0]) - 1);

And here:

               strpadcpy((char *)sysib.sysib_322.ext_names[0],
                         sizeof(sysib.sysib_322.ext_names[0]),
                         qemu_name, '\0');

>              } else {
>                  ebcdic_put(sysib.sysib_322.vm[0].name, "TCGguest", 8);
>                  strcpy((char *)sysib.sysib_322.ext_names[0], "TCGguest");
>