Re: CVE-2023-52451: powerpc/pseries/memhp: Fix access beyond end of drmem array

[Michal Hocko]

[Let me add Michael as PPC maintainer - the thread starts with
http://lkml.kernel.org/r/2024022257-CVE-2023-52451-7bdb@gregkh]

On Tue 27-02-24 06:14:45, Greg KH wrote:
> On Mon, Feb 26, 2024 at 05:36:57PM +0100, Michal Hocko wrote:
[...]
> > All that being said I dispute the issue fixed here has any more security
> > relevance than allowing untrusted user to control memory hotplug which
> > could easily result in DoS of the system.
> 
> Ok, I traced this call back, and here is the callpath, starting with a
> write to the the 'dlpar' sysfs file (conviently NOT documented in
> Documentation/ABI, and it looks like it violates the "one value per
> file" rule...)
> 	dlpar_store()
> 	handle_dlpar_errorlog()
> 	dlpar_memory()
> 	dlpar_memory_remove_by_index()
> 
> Yes, the kernel by default sets 'dlpar' to 0644, BUT that means that
> root in a container can cause this use-after-free to happen, or if the
> permissions are changed by userspace, or if you are in "lockdown mode",
> or if you want to attempt the crazy "confidential computing" model, or
> if you have a system which root is possible for some things by normal
> users (there are lots of different security models out there...)

This is all nice but please do realize that if you allow access to to
memory hotremove to any untrusted entity (be it a root in container or
by changing access permissions) then the machine is in a serious
resource management control trouble already and that is a security
threat already.

> Yes, I will argue that making the sysfs file writable by userspace is
> out of our control, but what is in our control is the fact that there is
> a out-of-bounds write that is fixed here, and we don't want those to be
> able to be triggered by anyone as that is a weakness in our codebase.

Yes, and that is why the fix is good and nobody disputes that. What I am
actually trying to drill down to is whether this is an actual security
threat worth assigning a CVE or it is just yet-anothing-pointless-CVE we
were so used to with the old process.

> That is what has caused the CVE to be created here, not the fact that
> root can remove memory as that's the normal expected operation to have
> happen here.
>
> However if the maintainer of the code here disputes this, we are more
> than willing to mark this invalid and reject the CVE.

Michael, do you see any real security risk being addressed by
bd68ffce69f6 ("powerpc/pseries/memhp: Fix access beyond end of drmem
array").

Thanks!
-- 
Michal Hocko
SUSE Labs