mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Reinette Chatre <reinette.chatre@intel.com>
To: Borislav Petkov <bp@alien8.de>
Cc: <tony.luck@intel.com>, <james.morse@arm.com>,
	<Dave.Martin@arm.com>, <babu.moger@amd.com>, <tglx@linutronix.de>,
	<dave.hansen@linux.intel.com>, <x86@kernel.org>, <hpa@zytor.com>,
	<ben.horgan@arm.com>, <fustini@kernel.org>, <fenghuay@nvidia.com>,
	<peternewman@google.com>, <yu.c.chen@intel.com>,
	<linux-kernel@vger.kernel.org>, <patches@lists.linux.dev>
Subject: Re: [PATCH v5 02/10] x86/resctrl: Protect against bad shift
Date: Wed, 22 Jul 2026 08:59:39 -0700	[thread overview]
Message-ID: <ae04714b-566e-4b21-ba6c-2f42c6400e87@intel.com> (raw)
In-Reply-To: <20260722005015.GKamATx-zEYGcRs2X4@fat_crate.local>

Hi Boris,

On 7/21/26 5:50 PM, Borislav Petkov wrote:
> On Tue, Jul 21, 2026 at 04:50:32PM -0700, Reinette Chatre wrote:
>> I see how such change could be considered absurd. I am ok to drop it from
>> the series and just keep treating it's report locally as a false positive.
> 
> Nah, but I'd like you, pls, to rewrite the commit message and explain more
> prominently
> 
> 1. resctrl does not support values than u32 max_bw
> 
> 2. what this is really fixing and where the potential issue can come from and
>    what the situation is today.
> 
> I'd like to have more background in the commit message why we even arrived at
> this patch.
> 
> I wouldnt've even done it - we have bigger fish to fry - but if you think it
> is important for resctrl, sure.
I am trying to help with bigger things in resctrl and find it easier to do so when
working on a baseline that is without implicit assumptions.

I plan to use changelog below that aims to address the items you highlighted and also
changed the math to make the bandwidth boundary where resctrl stop working clear:
	
	x86/resctrl: Protect against bad shift

	The size of the bandwidth field is enumerated from AMD hardware. resctrl uses   
	this field width to determine the maximum bandwidth supported that is stored in 
	resctrl_membw::max_bw. User space allocation requests ("control values") are    
	compared against this maximum for validity before being programmed to hardware. 
                                                                                
	resctrl filesystem and resctrl x86 architecture code only support u32 control   
	values: resctrl_membw::max_bw is a u32, the control value provided by user space
	is parsed into u32 local variables, and after validity checks, the control value
	is staged into the u32 resctrl_staged_config::new_ctrl for architecture         
	consumption. The resctrl x86 architecture code in turn caches the new control   
	value into the u32 array rdt_hw_ctrl_domain::ctrl_val[].                        
                                                                                
	The AMD bandwidth field to which control values are written can be up to 64 bits
	wide. While not an issue with current hardware (bandwidths that require more than
	a u32, more than 536870911.875 GB/s, seem unreasonable today), it is theoretically
	possible that enumeration of maximum bandwidth field width will return values   
	that are according to specification but cannot be supported by resctrl.         
                                                                                
	Static checkers complain about this size mismatch.                              
                                                                                
	Fix the static checker complaint by explicitly encoding the fact that resctrl is
	unable to support all values that the hardware specification allows. Switch to  
	BIT() instead of open-coding the bitshift to avoid signed integer overflow if   
	the number of bits is a valid 31.                        

Reinette

  reply	other threads:[~2026-07-22 15:59 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-01  4:27 [PATCH v5 00/10] x86,fs/resctrl: Improve resctrl quality and consistency Reinette Chatre
2026-07-01  4:27 ` [PATCH v5 01/10] fs/resctrl: Use correct format specifier for printing error pointers Reinette Chatre
2026-07-01  4:27 ` [PATCH v5 02/10] x86/resctrl: Protect against bad shift Reinette Chatre
2026-07-21  1:59   ` Borislav Petkov
2026-07-21 16:14     ` Reinette Chatre
2026-07-21 23:15       ` Borislav Petkov
2026-07-21 23:50         ` Reinette Chatre
2026-07-22  0:50           ` Borislav Petkov
2026-07-22 15:59             ` Reinette Chatre [this message]
2026-07-22 20:03               ` Borislav Petkov
2026-07-22 21:07                 ` Reinette Chatre
2026-07-22 21:11                 ` [PATCH v5.1 " Reinette Chatre
2026-07-01  4:27 ` [PATCH v5 03/10] fs/resctrl: Change pattern used to track number of entries in enum Reinette Chatre
2026-07-01  4:27 ` [PATCH v5 04/10] fs/resctrl: Use accurate type for rdt_resource::rid Reinette Chatre
2026-07-01  4:27 ` [PATCH v5 05/10] fs/resctrl: Pass error reading event through to user space Reinette Chatre
2026-07-01  4:27 ` [PATCH v5 06/10] fs/resctrl: Use accurate and symmetric exit flows Reinette Chatre
2026-07-01  4:27 ` [PATCH v5 07/10] fs/resctrl: Change last_cmd_status custom during input parsing Reinette Chatre
2026-07-01  4:27 ` [PATCH v5 08/10] fs/resctrl: Add last_cmd_status support for writes to max_threshold_occupancy Reinette Chatre
2026-07-01  4:27 ` [PATCH v5 09/10] fs/resctrl: Communicate resource group deleted error via last_cmd_status Reinette Chatre
2026-07-01  4:27 ` [PATCH v5 10/10] fs/resctrl: Inform user space when status buffer overflowed Reinette Chatre
2026-07-14 17:49 ` [PATCH v5 00/10] x86,fs/resctrl: Improve resctrl quality and consistency Reinette Chatre
2026-07-20 17:16   ` Borislav Petkov
2026-07-14 22:45 ` Moger, Babu
2026-07-15  0:00   ` Reinette Chatre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ae04714b-566e-4b21-ba6c-2f42c6400e87@intel.com \
    --to=reinette.chatre@intel.com \
    --cc=Dave.Martin@arm.com \
    --cc=babu.moger@amd.com \
    --cc=ben.horgan@arm.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=fenghuay@nvidia.com \
    --cc=fustini@kernel.org \
    --cc=hpa@zytor.com \
    --cc=james.morse@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=peternewman@google.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    --cc=yu.c.chen@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®