Re: [PATCH v1 1/2] arm_mpam: Fix a NULL pointer dereference on unbinding after an error interrupt

Hi Andre,

On 8/6/26 16:38, Andre Przywara wrote:
> Hi,
> 
> On 8/6/26 16:46, Ben Horgan wrote:
>> If a user unbinds an MSC after mpam_disable() has been run in response
>> to an error interrupt then a dereference of a NULL pointer occurs as
>> mpam_disable() sets the drvdata to NULL. Add an early return to the
>> driver
>> remove callback to avoid this.
> 
> Yes, I added a very similar patch to my stack yesterday, in response to
> a Sashiko complaint.
> However I was still calling mpam_free_garbage() at the end, to me it
> looks like this is not depending on any MSC?

mpam_free_garbage() doesn't depend on any MSC but it won't have any work
to do unless something in the driver has been destroyed and
mpam_disable() already calls mpam_free_garbage() after it calls the
_destroy() functions. Hence, I think it's ok either way.

Thanks,

Ben

> 
> Cheers,
> Andre
> 
>> Fixes: f04046f2577a ("arm_mpam: Add probe/remove for mpam msc driver
>> and kbuild boiler plate")
>> Signed-off-by: Ben Horgan <[email protected]>
>> ---
>>   drivers/resctrl/mpam_devices.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/
>> mpam_devices.c
>> index 2f09f4b78bd3..bc6cc0b5c96b 100644
>> --- a/drivers/resctrl/mpam_devices.c
>> +++ b/drivers/resctrl/mpam_devices.c
>> @@ -2025,6 +2025,9 @@ static void mpam_msc_drv_remove(struct
>> platform_device *pdev)
>>   {
>>       struct mpam_msc *msc = platform_get_drvdata(pdev);
>>   +    if (!msc)
>> +        return;
>> +
>>       mutex_lock(&mpam_list_lock);
>>       mpam_msc_destroy(msc);
>>       mutex_unlock(&mpam_list_lock);
>