OK.
It appears this is already handled safely in vmd and vmctl -- the string
always had a terminal NUL before copying in any direction, and since both
arrays are the same storage size, there is no way to lose the NUL, and
the struct being filled is allocated with PR_ZERO.
Tobias Heider <tobias.hei...@stusta.de> wrote:
> vmm uses 'strncpy(vm->vm_name, vcp->vcp_name, VMM_MAX_NAME_LEN)' to copy
> to buffers of size VMM_MAX_NAME_LEN, which can leave the resulting string
> unterminated.
> From strncpy(3):
> strncpy() only NUL terminates the destination string when the length of
> the source string is less than the length parameter.
>
> I propose replacing it with 'strlcpy' which does the right thing and
> only copies up to dstsize - 1 characters.
>
> ok?
>
> CID 1453255
>
> Index: sys/arch/amd64/amd64/vmm.c
> ===================================================================
> RCS file: /mount/openbsd/cvs/src/sys/arch/amd64/amd64/vmm.c,v
> retrieving revision 1.266
> diff -u -p -r1.266 vmm.c
> --- sys/arch/amd64/amd64/vmm.c 11 Mar 2020 16:38:42 -0000 1.266
> +++ sys/arch/amd64/amd64/vmm.c 12 Mar 2020 21:15:01 -0000
> @@ -1167,7 +1167,7 @@ vm_create(struct vm_create_params *vcp,
> memcpy(vm->vm_memranges, vcp->vcp_memranges,
> vm->vm_nmemranges * sizeof(vm->vm_memranges[0]));
> vm->vm_memory_size = memsize;
> - strncpy(vm->vm_name, vcp->vcp_name, VMM_MAX_NAME_LEN);
> + strlcpy(vm->vm_name, vcp->vcp_name, VMM_MAX_NAME_LEN);
>
> rw_enter_write(&vmm_softc->vm_lock);
>
> @@ -3718,7 +3718,7 @@ vm_get_info(struct vm_info_params *vip)
> out[i].vir_ncpus = vm->vm_vcpu_ct;
> out[i].vir_id = vm->vm_id;
> out[i].vir_creator_pid = vm->vm_creator_pid;
> - strncpy(out[i].vir_name, vm->vm_name, VMM_MAX_NAME_LEN);
> + strlcpy(out[i].vir_name, vm->vm_name, VMM_MAX_NAME_LEN);
> rw_enter_read(&vm->vm_vcpu_lock);
> for (j = 0; j < vm->vm_vcpu_ct; j++) {
> out[i].vir_vcpu_state[j] = VCPU_STATE_UNKNOWN;
>
