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

[David Laight <david.laight.linux@gmail.com>]

On Thu, 18 Jun 2026 15:24:17 +0200
Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> wrote:

> On 6/8/26 11:55 AM, david.laight.linux@gmail.com wrote:
> > From: David Laight <david.laight.linux@gmail.com>
> > 
> > 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.
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).

	David

> 
> Konrad