Re: [PATCH next] drivers/rpmsg: Fix copy of channel->name into open request

[Konrad Dybcio]

On 6/18/26 4:59 PM, David Laight wrote:
> On Thu, 18 Jun 2026 15:24:17 +0200
> Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx> wrote:
> 
>> On 6/8/26 11:55 AM, david.laight.linux@xxxxxxxxx wrote:
>>> From: David Laight <david.laight.linux@xxxxxxxxx>
>>>
>>> Nothing obvious ensures that the name is less than GLINK_CMD_OPEN (32)  
>> 						     ^ GLINK_NAME_SIZE
> 
> I was writing a lot of commit messages, most with -m 'text'.
> 
>> [...]
>>
>>> @@ -481,8 +481,7 @@ static int qcom_glink_send_open_req(struct qcom_glink *glink,
>>>  				    struct glink_channel *channel)
>>>  {
>>>  	DEFINE_RAW_FLEX(struct glink_msg, req, data, GLINK_NAME_SIZE);
>>> -	int name_len = strlen(channel->name) + 1;
>>> -	int req_len = ALIGN(sizeof(*req) + name_len, 8);
>>> +	int name_len, req_len;
>>>  	int ret;
>>>  	unsigned long flags;
>>>  
>>> @@ -498,14 +497,20 @@ static int qcom_glink_send_open_req(struct qcom_glink *glink,
>>>  
>>>  	channel->lcid = ret;
>>>  
>>> +	name_len = strscpy_pad(req->data, channel->name, GLINK_NAME_SIZE);
>>> +	if (name_len < 0)
>>> +		name_len = GLINK_NAME_SIZE;
>>> +	else
>>> +		name_len++;  
>>
>> Should we perhaps do something along the lines of:
>>
>> WARN_ON(strlen(name) > GLINK_NAME_SIZE)
>>
>> to prevent silent clipping?
> 
> strscpy() tells you whether the copy got truncated.
> No point calling strlen() again.

Right.

> But I'm not really sure it is worth it.
> Any length check of user-supplied names should be much earlier,
> this is just ensuring this code doesn't overwrite its own stack.
> (and ensuring stale stack doesn't get sent as padding).

Fair. I'll check that out separately.

Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>

Konrad