On Sat, Mar 28, 2020 at 06:47:47PM -0600, Theo de Raadt wrote:
> Or strncpy with length - 1 would be also good, since it won't copy
>foo\0bar\0
> fully, but only
>foo\0
> into the buffer and store it as
>foo\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0
> and gaurantee the \0 on the
Or strncpy with length - 1 would be also good, since it won't copy
foo\0bar\0
fully, but only
foo\0
into the buffer and store it as
foo\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0
and gaurantee the \0 on the in-kernel buffer.
On Sat, Mar 28, 2020 at 05:33:05PM -0600, Theo de Raadt wrote:
> Pretty obvious why.
>
> The kernel doesn't check it's a string, before calling strlcpy
> which (correctly) runs off the array hunting for the terminal NUL,
> and into the next object, and I guess it finds a NUL in the next
> VA page
Pretty obvious why.
The kernel doesn't check it's a string, before calling strlcpy
which (correctly) runs off the array hunting for the terminal NUL,
and into the next object, and I guess it finds a NUL in the next
VA page which isn't actually mapped with storage.
With strncpy, this was safe.
> 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 - 1.266
> +++
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
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 wrote:
