On Thu, Mar 12, 2020 at 10:31:13PM +0100, Tobias Heider 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?
>
good find. Thanks!
> 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;
>
