Re: [tip:x86/urgent] BUILD SUCCESS WITH WARNING 064855a69003c24bd6b473b367d364e418c57625

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Borislav and Babu,

On 8/19/2021 2:00 AM, Borislav Petkov wrote:
> On Thu, Aug 19, 2021 at 02:15:16PM +0800, Chen, Rong A wrote:
>>
>>
>> On 8/15/2021 4:42 PM, Borislav Petkov wrote:
>>> Hi,
>>>
>>> On Sun, Aug 15, 2021 at 04:36:40PM +0800, kernel test robot wrote:
>>>> tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/urgent
>>>> branch HEAD: 064855a69003c24bd6b473b367d364e418c57625  x86/resctrl: Fix default monitoring groups reporting
>>>>
>>>> possible Warning in current branch:
>>>>
>>>> arch/x86/kernel/cpu/resctrl/monitor.c:310 __mon_event_count() error: uninitialized symbol 'm'.
>>>> arch/x86/kernel/cpu/resctrl/monitor.c:315 __mon_event_count() error: potentially dereferencing uninitialized 'm'.
>>>>
>>>> Warning ids grouped by kconfigs:
>>>>
>>>> gcc_recent_errors
>>>> `-- i386-randconfig-m021-20210812
>>>>       |-- arch-x86-kernel-cpu-resctrl-monitor.c-__mon_event_count()-error:potentially-dereferencing-uninitialized-m-.
>>>>       `-- arch-x86-kernel-cpu-resctrl-monitor.c-__mon_event_count()-error:uninitialized-symbol-m-.
>>>
>>> AFAIR, I had already asked you guys to make those reports more useful
>>> as, for example, adding a link to that randconfig above or even
>>> attaching it so that a person - not a machine - reading it, can
>>> *actually* act upon it.
>>>
>>> But that hasn't happened.
>>>
>>> Until it happens, I'm going to ignore all those reports from you.
>>>
>>
>> Hi Borislav,
>>
>> Sorry about it, the actual link is at https://lists.01.org/hyperkitty/list/kbuild@lists.01.org/thread/PZVMY3VJU4QY4HQXHK3MLPQ2KZ5CNAYH/,
>> and it's still an internal report that the robot don't know whether it's a
>> false positive or not, we'll update the mail contents to avoid
>> misunderstanding.
> 
> Aha, ok, smatch thinks that m might not be initialized because
> 
> 064855a69003 ("x86/resctrl: Fix default monitoring groups reporting")
> 
> removed the default case:
> 
> -       default:
> -               /*
> -                * Code would never reach here because
> -                * an invalid event id would fail the __rmid_read.
> -                */
> -               return -EINVAL;
> 
> I'm guessing that comment which got removed too, explains why that's ok.
> 
> Adding folks to Cc.

I can confirm that the removed comment explains why m would be 
initialized when used in the code that follows.

How would you prefer to address this? We could add just the comment back 
in support of future reports or perhaps by adding the default case back 
with the same error that would be returned earlier when there is an 
invalid EVENT_ID. Something like:

---8<---
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c 
b/arch/x86/kernel/cpu/resctrl/monitor.c
index 57e4bb695ff9..05b99e4d621c 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -304,6 +304,12 @@ static u64 __mon_event_count(u32 rmid, struct 
rmid_read *rr)
  	case QOS_L3_MBM_LOCAL_EVENT_ID:
  		m = &rr->d->mbm_local[rmid];
  		break;
+	default:
+		/*
+		 * Code would never reach here because
+		 * an invalid event id would fail the __rmid_read.
+		 */
+		return RMID_VAL_ERROR;
  	}

  	if (rr->first) {