Re: [PATCH v1 07/11] arm_mpam: Initialize all of struct mon_read in mpam_restore_mbwu_state()

[Ben Horgan]

Hi Lee,

On 7/21/26 00:05, Lee Trager wrote:
> On 7/10/26 4:55 AM, Ben Horgan wrote:
> 
>> m->err may be read before initialization in __ris_msmon_read() when called
>> from mpam_restore_mbwu_state().
>>
>> Initialize the whole struct mon_read in mpam_restore_mbwu_state() and fix
>> the spelling of mbwu in the name.
>>
>> Fixes: 41e8a14950e1 ("arm_mpam: Track bandwidth counter state for power management")
>> Signed-off-by: Ben Horgan <ben.horgan@xxxxxxx>
>> ---
>>   drivers/resctrl/mpam_devices.c | 13 +++++++------
>>   1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
>> index a49f426aefc0..c9adc450f087 100644
>> --- a/drivers/resctrl/mpam_devices.c
>> +++ b/drivers/resctrl/mpam_devices.c
>> @@ -1640,7 +1640,6 @@ static int mpam_restore_mbwu_state(void *_ris)
>>   {
>>       int i;
>>       u64 val;
> val is still uninitialized. Its passed to to __ris_mon_read() below which does *m->val += now;

Ok, I don't think this causes any actual problems as an unitialized automatic variable has an
indeterminate value, we're just adding to it rather than making any decisions or persisting the
value. As it's unsigned the addition is defined even if it wraps around. Having said that, it's
clearer just to initialize it. I'll set it to 0.

Thanks,

Ben

>> -    struct mon_read mwbu_arg;
>>       struct mpam_msc_ris *ris = _ris;
>>       struct msmon_mbwu_state *mbwu_state;
>>       struct mpam_msc *msc = ris->vmsc->msc;
>> @@ -1653,16 +1652,18 @@ static int mpam_restore_mbwu_state(void *_ris)
>>               return -EIO;
>>             if (ris->mbwu_state[i].enabled) {
>> -            mwbu_arg.ris = ris;
>> -            mwbu_arg.ctx = &ris->mbwu_state[i].cfg;
>> -            mwbu_arg.type = mpam_msmon_choose_counter(class);
>> -            mwbu_arg.val = &val;
>> +            struct mon_read mbwu_arg = {
>> +                .ris = ris,
>> +                .ctx = &ris->mbwu_state[i].cfg,
>> +                .type = mpam_msmon_choose_counter(class),
>> +                .val = &val
>> +            };
>>                 mbwu_state->reset_on_next_read = true;
>>                 mpam_mon_sel_unlock(msc);
>>   -            __ris_msmon_read(&mwbu_arg);
>> +            __ris_msmon_read(&mbwu_arg);
>>           } else {
>>               mpam_mon_sel_unlock(msc);
>>           }