[PATCH v1 0/2] MPAM Fixes for UAF and NPE on unbind

[PATCH v1 0/2] MPAM Fixes for UAF and NPE on unbind

[Ben Horgan]

The MPAM driver expects to manage all the MSC in a system. At boot it
delays its initialization until it has probed all of them but once
running it is unprepared to have the MSC taken away and that can trigger
UAF and NPE. Workaround this by ensuring it never relinquishes the MSC.

Ben Horgan (2):
  arm_mpam: Fix a NULL pointer dereference on unbinding after an error
    interrupt
  arm_mpam: Disable driver unbind to avoid UAF

 drivers/resctrl/mpam_devices.c | 4 ++++
 1 file changed, 4 insertions(+)

-- 
2.43.0

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

[Ben Horgan]

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.

Fixes: f04046f2577a ("arm_mpam: Add probe/remove for mpam msc driver and kbuild boiler plate")
Signed-off-by: Ben Horgan <ben.horgan@arm.com>
---
 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);
-- 
2.43.0

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

[Andre Przywara]

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?

Cheers,
Andre

> Fixes: f04046f2577a ("arm_mpam: Add probe/remove for mpam msc driver and kbuild boiler plate")
> Signed-off-by: Ben Horgan <ben.horgan@arm.com>
> ---
>   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);

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

[Ben Horgan]

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 <ben.horgan@arm.com>
>> ---
>>   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);
> 

[PATCH v1 2/2] arm_mpam: Disable driver unbind to avoid UAF

[Ben Horgan]

When a user unbinds an MSC and that MSC is the only MSC left for a
component then the corresponding mpam_component will be freed. If the user
then goes on to read the schemata file in the resctrl filesystem then the
mpam_component will be accessed from resctrl_arch_get_config() leading to a
use after free.

As the MPAM driver is not a module the unbind sysfs interface is the only
way to trigger the remove. Instead of dealing with the complexity of
allowing some unused MSC to unbind just remove the unbind sysfs interface.

Fixes: f04046f2577a ("arm_mpam: Add probe/remove for mpam msc driver and kbuild boiler plate")
Signed-off-by: Ben Horgan <ben.horgan@arm.com>
---
 drivers/resctrl/mpam_devices.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index bc6cc0b5c96b..e888de8d4896 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -2141,6 +2141,7 @@ static int mpam_msc_drv_probe(struct platform_device *pdev)
 static struct platform_driver mpam_msc_driver = {
 	.driver = {
 		.name = "mpam_msc",
+		.suppress_bind_attrs = true,
 	},
 	.probe = mpam_msc_drv_probe,
 	.remove = mpam_msc_drv_remove,
-- 
2.43.0