The fastrpc_buf_free function currently does not handle the case where
the input buffer pointer (buf) is NULL. This can lead to a null pointer
dereference, causing a crash or undefined behavior when the function
attempts to access members of the buf structure. Add a NULL check to
ensure safe handling of NULL pointers and prevent potential crashes.

Fixes: c68cfb718c8f9 ("misc: fastrpc: Add support for context Invoke method")
Cc: stable@kernel.org
Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 7b7a22c91fe4..ca3721365ddc 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -394,6 +394,9 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
 
 static void fastrpc_buf_free(struct fastrpc_buf *buf)
 {
+	if (!buf)
+		return;
+
 	dma_free_coherent(buf->dev, buf->size, buf->virt,
 			  FASTRPC_PHYS(buf->phys));
 	kfree(buf);
-- 
2.34.1

On 5/13/25 05:28, Ekansh Gupta wrote:
> The fastrpc_buf_free function currently does not handle the case where
> the input buffer pointer (buf) is NULL. This can lead to a null pointer
> dereference, causing a crash or undefined behavior when the function
> attempts to access members of the buf structure. Add a NULL check to
> ensure safe handling of NULL pointers and prevent potential crashes.
> 
You are mostly defining the code here, but not the root cause of it,
What exactly is the call trace for this crash?

> Fixes: c68cfb718c8f9 ("misc: fastrpc: Add support for context Invoke method")
> Cc: stable@kernel.org
> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
> ---
>  drivers/misc/fastrpc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 7b7a22c91fe4..ca3721365ddc 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -394,6 +394,9 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
>  
>  static void fastrpc_buf_free(struct fastrpc_buf *buf)
>  {
> +	if (!buf)
> +		return;
> +
Most of the users of the fastrpc_buf_free() already have the null
checks, It will be Interesting to know.

If we decide to make this function to do null null check, then the
existing checks in the caller are redundant.

--srini
>  	dma_free_coherent(buf->dev, buf->size, buf->virt,
>  			  FASTRPC_PHYS(buf->phys));
>  	kfree(buf);

On Mon, May 19, 2025 at 10:25:46AM +0100, Srinivas Kandagatla wrote:
> On 5/13/25 05:28, Ekansh Gupta wrote:
> > The fastrpc_buf_free function currently does not handle the case where
> > the input buffer pointer (buf) is NULL. This can lead to a null pointer
> > dereference, causing a crash or undefined behavior when the function
> > attempts to access members of the buf structure. Add a NULL check to
> > ensure safe handling of NULL pointers and prevent potential crashes.
> > 
> You are mostly defining the code here, but not the root cause of it,
> What exactly is the call trace for this crash?
> 
> > Fixes: c68cfb718c8f9 ("misc: fastrpc: Add support for context Invoke method")
> > Cc: stable@kernel.org
> > Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
> > ---
> >  drivers/misc/fastrpc.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> > index 7b7a22c91fe4..ca3721365ddc 100644
> > --- a/drivers/misc/fastrpc.c
> > +++ b/drivers/misc/fastrpc.c
> > @@ -394,6 +394,9 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
> >  
> >  static void fastrpc_buf_free(struct fastrpc_buf *buf)
> >  {
> > +	if (!buf)
> > +		return;
> > +
> Most of the users of the fastrpc_buf_free() already have the null
> checks, It will be Interesting to know.
> 
> If we decide to make this function to do null null check, then the
> existing checks in the caller are redundant.

I think it was a primary reason for a change: to eliminate NULL checks
on the caller side, as we do in a lot of other kernel API.

> 
> --srini
> >  	dma_free_coherent(buf->dev, buf->size, buf->virt,
> >  			  FASTRPC_PHYS(buf->phys));
> >  	kfree(buf);
> 

-- 
With best wishes
Dmitry

On 5/19/25 11:09, Dmitry Baryshkov wrote:
> On Mon, May 19, 2025 at 10:25:46AM +0100, Srinivas Kandagatla wrote:
>> On 5/13/25 05:28, Ekansh Gupta wrote:
>>> The fastrpc_buf_free function currently does not handle the case where
>>> the input buffer pointer (buf) is NULL. This can lead to a null pointer
>>> dereference, causing a crash or undefined behavior when the function
>>> attempts to access members of the buf structure. Add a NULL check to
>>> ensure safe handling of NULL pointers and prevent potential crashes.
>>>
>> You are mostly defining the code here, but not the root cause of it,
>> What exactly is the call trace for this crash?
>>
>>> Fixes: c68cfb718c8f9 ("misc: fastrpc: Add support for context Invoke method")
>>> Cc: stable@kernel.org
>>> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
>>> ---
>>>  drivers/misc/fastrpc.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>>> index 7b7a22c91fe4..ca3721365ddc 100644
>>> --- a/drivers/misc/fastrpc.c
>>> +++ b/drivers/misc/fastrpc.c
>>> @@ -394,6 +394,9 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
>>>  
>>>  static void fastrpc_buf_free(struct fastrpc_buf *buf)
>>>  {
>>> +	if (!buf)
>>> +		return;
>>> +
>> Most of the users of the fastrpc_buf_free() already have the null
>> checks, It will be Interesting to know.
>>
>> If we decide to make this function to do null null check, then the
>> existing checks in the caller are redundant.
> 
> I think it was a primary reason for a change: to eliminate NULL checks
> on the caller side, as we do in a lot of other kernel API.

Lets remove the existing NULL checks at caller side as part of this
patch too.


--Srini

> 
>>
>> --srini
>>>  	dma_free_coherent(buf->dev, buf->size, buf->virt,
>>>  			  FASTRPC_PHYS(buf->phys));
>>>  	kfree(buf);
>>
>

On 5/19/2025 4:10 PM, Srinivas Kandagatla wrote:
> On 5/19/25 11:09, Dmitry Baryshkov wrote:
>> On Mon, May 19, 2025 at 10:25:46AM +0100, Srinivas Kandagatla wrote:
>>> On 5/13/25 05:28, Ekansh Gupta wrote:
>>>> The fastrpc_buf_free function currently does not handle the case where
>>>> the input buffer pointer (buf) is NULL. This can lead to a null pointer
>>>> dereference, causing a crash or undefined behavior when the function
>>>> attempts to access members of the buf structure. Add a NULL check to
>>>> ensure safe handling of NULL pointers and prevent potential crashes.
>>>>
>>> You are mostly defining the code here, but not the root cause of it,
>>> What exactly is the call trace for this crash?
>>>
>>>> Fixes: c68cfb718c8f9 ("misc: fastrpc: Add support for context Invoke method")
>>>> Cc: stable@kernel.org
>>>> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
>>>> ---
>>>>  drivers/misc/fastrpc.c | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>>>> index 7b7a22c91fe4..ca3721365ddc 100644
>>>> --- a/drivers/misc/fastrpc.c
>>>> +++ b/drivers/misc/fastrpc.c
>>>> @@ -394,6 +394,9 @@ static int fastrpc_map_lookup(struct fastrpc_user *fl, int fd,
>>>>  
>>>>  static void fastrpc_buf_free(struct fastrpc_buf *buf)
>>>>  {
>>>> +	if (!buf)
>>>> +		return;
>>>> +
>>> Most of the users of the fastrpc_buf_free() already have the null
>>> checks, It will be Interesting to know.
>>>
>>> If we decide to make this function to do null null check, then the
>>> existing checks in the caller are redundant.
>> I think it was a primary reason for a change: to eliminate NULL checks
>> on the caller side, as we do in a lot of other kernel API.
> Lets remove the existing NULL checks at caller side as part of this
> patch too.
Sure, I'll remove the checks in the next spin.
>
>
> --Srini
>
>>> --srini
>>>>  	dma_free_coherent(buf->dev, buf->size, buf->virt,
>>>>  			  FASTRPC_PHYS(buf->phys));
>>>>  	kfree(buf);