Re: [PATCH v1 1/5] misc: fastrpc: Add NULL check to fastrpc_buf_free to prevent crash

[Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>]

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);