On 09.06.2025 21:15, Kees Cook wrote:
>
> I think we need wholesale fix the kernel's usage of
> strncpy_from_user()... first, how is it being used?
So I looked at some of the codes under driver, but it's not very clear
to me how they are currently used.
For example in drivers/tty/vt/vt.c there is this code
if (!op->data)
s = NULL;
else if (strncpy_from_user(name, op->data, MAX_FONT_NAME - 1) < 0)
return -EFAULT;
else
name[MAX_FONT_NAME - 1] = 0;
Which looks like it's not getting NUL terminated.
Where as there's also other instances such as in
drivers/accel/ivpu/ivpu_debugfs.c
ret = strncpy_from_user(buffer, user_buf, size);
if (ret < 0)
return ret;
and in drivers/gpu/drm/virtio/virtgpu_ioctl.c
ret = strncpy_from_user(vfpriv->debug_name,
u64_to_user_ptr(value),
DEBUG_NAME_MAX_LEN - 1);
again in drivers/dma/xilinx/xilinx_dpdma.c we see
ret = strncpy_from_user(kern_buff, buf, size);
if (ret < 0)
goto done;
So it seems like it's not very consistent to me. But I might be wrong.
> Are things being
> manually terminated? Are something destinations not actually C Strings?
Some of them are, while others are not. For example in
drivers/dma/xilinx/xilinx_dpdma.c example the kern_buff is a pointer to
char while in the drivers/gpu/drm/virtio/virtgpu_ioctl.c example
vfpriv->debug_name is an array.
> We may want two APIs (like strtomem vs strscpy). And then since we're
> dealing with user data, I would think padding should be included?
Can you please example on this a bit, like would you like two separate
functions, for example strscpy_from_user and strtomem_from_user. For the
padding we use _pad version of the strscpy function.
> --
> Kees Cook
--
Regards,
listout
