[PATCH v1] misc: fastrpc: Fix channel resource access in device_open

Ekansh Gupta posted 1 patch 7 months ago
There is a newer version of this series
drivers/misc/fastrpc.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
[PATCH v1] misc: fastrpc: Fix channel resource access in device_open
Posted by Ekansh Gupta 7 months ago
During rpmsg_probe, fastrpc device nodes are created first, then
channel specific resources are initialized, followed by
of_platform_populate, which triggers context bank probing. This
sequence can cause issues as applications might open the device
node before channel resources are initialized or the session is
available, leading to problems. For example, spin_lock is initialized
after the device node creation, but it is used in device_open,
potentially before initialization. Add a check in device_open for
rpdev and update rpdev at the end of rpmsg_probe to resources are
available before applications allocate sessions.

Fixes: f6f9279f2bf0e ("misc: fastrpc: Add Qualcomm fastrpc basic driver model")
Cc: stable@kernel.org
Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 7b7a22c91fe4..40c7fa048ba7 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1568,6 +1568,9 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp)
 	fdevice = miscdev_to_fdevice(filp->private_data);
 	cctx = fdevice->cctx;
 
+	if (!cctx->rpdev)
+		return -ENODEV;
+
 	fl = kzalloc(sizeof(*fl), GFP_KERNEL);
 	if (!fl)
 		return -ENOMEM;
@@ -2363,12 +2366,13 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 	spin_lock_init(&data->lock);
 	idr_init(&data->ctx_idr);
 	data->domain_id = domain_id;
-	data->rpdev = rpdev;
 
 	err = of_platform_populate(rdev->of_node, NULL, NULL, rdev);
 	if (err)
 		goto populate_error;
 
+	data->rpdev = rpdev;
+
 	return 0;
 
 populate_error:
-- 
2.34.1
Re: [PATCH v1] misc: fastrpc: Fix channel resource access in device_open
Posted by Dmitry Baryshkov 7 months ago
On Sat, May 17, 2025 at 12:54:32PM +0530, Ekansh Gupta wrote:
> During rpmsg_probe, fastrpc device nodes are created first, then
> channel specific resources are initialized, followed by
> of_platform_populate, which triggers context bank probing. This
> sequence can cause issues as applications might open the device
> node before channel resources are initialized or the session is
> available, leading to problems. For example, spin_lock is initialized
> after the device node creation, but it is used in device_open,
> potentially before initialization. Add a check in device_open for
> rpdev and update rpdev at the end of rpmsg_probe to resources are
> available before applications allocate sessions.

Can we fix this by registering the device node after initializing
channel resources?

> 
> Fixes: f6f9279f2bf0e ("misc: fastrpc: Add Qualcomm fastrpc basic driver model")
> Cc: stable@kernel.org
> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
> ---
>  drivers/misc/fastrpc.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 7b7a22c91fe4..40c7fa048ba7 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -1568,6 +1568,9 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp)
>  	fdevice = miscdev_to_fdevice(filp->private_data);
>  	cctx = fdevice->cctx;
>  
> +	if (!cctx->rpdev)
> +		return -ENODEV;
> +
>  	fl = kzalloc(sizeof(*fl), GFP_KERNEL);
>  	if (!fl)
>  		return -ENOMEM;
> @@ -2363,12 +2366,13 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  	spin_lock_init(&data->lock);
>  	idr_init(&data->ctx_idr);
>  	data->domain_id = domain_id;
> -	data->rpdev = rpdev;
>  
>  	err = of_platform_populate(rdev->of_node, NULL, NULL, rdev);
>  	if (err)
>  		goto populate_error;
>  
> +	data->rpdev = rpdev;
> +
>  	return 0;
>  
>  populate_error:
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry
Re: [PATCH v1] misc: fastrpc: Fix channel resource access in device_open
Posted by Ekansh Gupta 7 months ago

On 5/18/2025 4:33 PM, Dmitry Baryshkov wrote:
> On Sat, May 17, 2025 at 12:54:32PM +0530, Ekansh Gupta wrote:
>> During rpmsg_probe, fastrpc device nodes are created first, then
>> channel specific resources are initialized, followed by
>> of_platform_populate, which triggers context bank probing. This
>> sequence can cause issues as applications might open the device
>> node before channel resources are initialized or the session is
>> available, leading to problems. For example, spin_lock is initialized
>> after the device node creation, but it is used in device_open,
>> potentially before initialization. Add a check in device_open for
>> rpdev and update rpdev at the end of rpmsg_probe to resources are
>> available before applications allocate sessions.
> Can we fix this by registering the device node after initializing
> channel resources?
Ack.

Thanks.
>
>> Fixes: f6f9279f2bf0e ("misc: fastrpc: Add Qualcomm fastrpc basic driver model")
>> Cc: stable@kernel.org
>> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
>> ---
>>  drivers/misc/fastrpc.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index 7b7a22c91fe4..40c7fa048ba7 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -1568,6 +1568,9 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp)
>>  	fdevice = miscdev_to_fdevice(filp->private_data);
>>  	cctx = fdevice->cctx;
>>  
>> +	if (!cctx->rpdev)
>> +		return -ENODEV;
>> +
>>  	fl = kzalloc(sizeof(*fl), GFP_KERNEL);
>>  	if (!fl)
>>  		return -ENOMEM;
>> @@ -2363,12 +2366,13 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>>  	spin_lock_init(&data->lock);
>>  	idr_init(&data->ctx_idr);
>>  	data->domain_id = domain_id;
>> -	data->rpdev = rpdev;
>>  
>>  	err = of_platform_populate(rdev->of_node, NULL, NULL, rdev);
>>  	if (err)
>>  		goto populate_error;
>>  
>> +	data->rpdev = rpdev;
>> +
>>  	return 0;
>>  
>>  populate_error:
>> -- 
>> 2.34.1
>>