On Tue, Jul 2, 2024 at 9:04 PM Muhammad Qasim Abdul Majeed <qasim.majeed20@xxxxxxxxx> wrote: > > Replacing strcpy with strscpy and memory bound the copy. Why? In this particular case, it is not fundamentally necessary. > strcpy is a deprecated function. It should be removed from the kernel source. If the goal is to get rid of all strcpy() calls from the kernel because using it is generally unsafe, just say so in the changelog and it will be fine. > Reference: https://github.com/KSPP/linux/issues/88 > > Signed-off-by: Muhammad Qasim Abdul Majeed <qasim.majeed20@xxxxxxxxx> > > > In what way exactly is it vulnerable? > strcpy is a deprecated interface (reference: https://github.com/KSPP/linux/issues/88). It should be removed from kernel source. > It is reported as vulnerable in Enabling Linux in Safety Critical Applications (ELISA) builder. > > > Why is a runtime check needed here if all of the sizes in question are known at compile time? > Runtime check has been replaced with compile time check. > > --- > v1 -> v2: Commit message has been updated and runtime check is replace with compile time check. > > drivers/acpi/acpi_video.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c > index 1fda30388297..be8346a66374 100644 > --- a/drivers/acpi/acpi_video.c > +++ b/drivers/acpi/acpi_video.c > @@ -1128,8 +1128,8 @@ static int acpi_video_bus_get_one_device(struct acpi_device *device, void *arg) > return -ENOMEM; > } > > - strcpy(acpi_device_name(device), ACPI_VIDEO_DEVICE_NAME); > - strcpy(acpi_device_class(device), ACPI_VIDEO_CLASS); > + strscpy(acpi_device_name(device), ACPI_VIDEO_DEVICE_NAME, sizeof(ACPI_VIDEO_DEVICE_NAME)); > + strscpy(acpi_device_class(device), ACPI_VIDEO_CLASS, sizeof(ACPI_VIDEO_CLASS)); The strscpy() kerneldoc comment says this: * The size argument @... is only required when @dst is not an array, or * when the copy needs to be smaller than sizeof(@dst). So is it necessary to use the size argument here and below? > data->device_id = device_id; > data->video = video; > @@ -2010,8 +2010,8 @@ static int acpi_video_bus_add(struct acpi_device *device) > } > > video->device = device; > - strcpy(acpi_device_name(device), ACPI_VIDEO_BUS_NAME); > - strcpy(acpi_device_class(device), ACPI_VIDEO_CLASS); > + strscpy(acpi_device_name(device), ACPI_VIDEO_BUS_NAME, sizeof(ACPI_VIDEO_BUS_NAME)); > + strscpy(acpi_device_class(device), ACPI_VIDEO_CLASS, sizeof(ACPI_VIDEO_CLASS)); > device->driver_data = video; > > acpi_video_bus_find_cap(video); > --
