Re: [PATCH] drm/i915/gvt: use strscpy() instead of strcpy() in virt_vbt_generation()

[Jani Nikula]

On Tue, 01 Sep 2026, David Laight <david.laight.linux@xxxxxxxxx> wrote:
> On Tue,  1 Sep 2026 10:32:41 +0530
> Hrushiraj Gandhi <hrushirajg23@xxxxxxxxx> wrote:
>
>> strcpy() has no bound on the destination buffer, so convert this to
>> the bounded, always-NUL-terminating strscpy() instead. The literal
>> "BIOS_DATA_BLOCK" (15 chars + NUL) fits the 16-byte signature field
>> exactly, so this is a no-op change in behaviour.
>> 
>> No functional change.
>> 
>> Signed-off-by: Hrushiraj Gandhi <hrushirajg23@xxxxxxxxx>
>> ---
>>  drivers/gpu/drm/i915/gvt/opregion.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/gvt/opregion.c b/drivers/gpu/drm/i915/gvt/opregion.c
>> index d6e76ba31d60..f2527e26cd1e 100644
>> --- a/drivers/gpu/drm/i915/gvt/opregion.c
>> +++ b/drivers/gpu/drm/i915/gvt/opregion.c
>> @@ -151,7 +151,8 @@ static void virt_vbt_generation(struct vbt *v)
>>  	v->header.vbt_size = sizeof(struct vbt);
>>  	v->header.bdb_offset = offsetof(struct vbt, bdb_header);
>>  
>> -	strcpy(&v->bdb_header.signature[0], "BIOS_DATA_BLOCK");
>> +	strscpy(v->bdb_header.signature, "BIOS_DATA_BLOCK",
>> +		sizeof(v->bdb_header.signature));
>
> Pointless and potentially wrong.
> Both normally reduce to the same memcpy() call.
> If the fixed string is too long strcpy() generates a compile error
> whereas strscpy() will silently truncate.

Moreover, there's no provision that the signature must be NUL
terminated. All consumers must treat it as a 16-byte block which may or
may not be NUL terminated. In fact, it is usually padded with space
rather than NUL terminated, and one could argue the NUL termination is
wrong here.


BR,
Jani.

>
> (The '&' and '[0]' might need removing.)
>
> David
>
>>  	v->bdb_header.version = 186; /* child_dev_size = 33 */
>>  	v->bdb_header.header_size = sizeof(v->bdb_header);
>>  
>> 
>

-- 
Jani Nikula, Intel