Re: [PATCH v3 06/16] arm_mpam: propagate MSC read errors for __ris_msmon_read()

[Ben Horgan]

Hi Lee,

On 7/15/26 20:52, Lee Trager wrote:
> On 7/10/26 7:45 AM, Andre Przywara wrote:
> 
>> @@ -1748,6 +1758,7 @@ static int mpam_restore_mbwu_state(void *_ris)
>>   {
>>       int i;
>>       u64 val;
>> +    int ret = 0;
>>       struct mon_read mwbu_arg;
>>       struct mpam_msc_ris *ris = _ris;
>>       struct mpam_class *class = ris->vmsc->comp->class;
>> @@ -1760,10 +1771,14 @@ static int mpam_restore_mbwu_state(void *_ris)
>>               mwbu_arg.val = &val;
>>                 __ris_msmon_read(&mwbu_arg);
>> +            if (mwbu_arg.err) {
>> +                ret = mwbu_arg.err;
>> +                break;
>> +            }
>>           }
>>       }
>>   -    return 0;
>> +    return ret;
>>   }
>>     /* Call with MSC cfg_lock held */
> 
> 
> mwbu_arg is never initialized and __ris_msmon_read() only writes ->err on failure. If on a
> successful read mwb_arg.err tests stack garbage it can spuriously break ot of the loop, leaving the
> remaining monitors unrestored, and return a junk error code.
> 
> Setting `struct mon_read mwbu_arg = {};` matches what mpam_msmon_read() already does. Additionally
> val should be initialized to 0 too. __ris_msmon_reaD() does *m->val += now, and although the result
> is discarded here, KMSAN would flag the uninitialized read.
> 

Thanks for the bug report.

I have a proposed change here which aims to address this existing issue, [1]. Please could you check
and confirm whether this fixes the problem.

[1] https://lore.kernel.org/lkml/20260710115546.29644-8-ben.horgan@xxxxxxx/

Thanks,

Ben