CVE-2023-52685: pstore: ram_core: fix possible overflow in persistent_ram_init_ecc()

CVE-2023-52685: pstore: ram_core: fix possible overflow in persistent_ram_init_ecc()

[Greg Kroah-Hartman]

Description
===========

In the Linux kernel, the following vulnerability has been resolved:

pstore: ram_core: fix possible overflow in persistent_ram_init_ecc()

In persistent_ram_init_ecc(), on 64-bit arches DIV_ROUND_UP() will return
64-bit value since persistent_ram_zone::buffer_size has type size_t which
is derived from the 64-bit *unsigned long*, while the ecc_blocks variable
this value gets assigned to has (always 32-bit) *int* type.  Even if that
value fits into *int* type, an overflow is still possible when calculating
the size_t typed ecc_total variable further below since there's no cast to
any 64-bit type before multiplication.  Declaring the ecc_blocks variable
as *size_t* should fix this mess...

Found by Linux Verification Center (linuxtesting.org) with the SVACE static
analysis tool.

The Linux kernel CVE team has assigned CVE-2023-52685 to this issue.


Affected and fixed versions
===========================

	Issue introduced in 3.4 with commit 9cc05ad97c57 and fixed in 4.19.306 with commit 3b333cded94f
	Issue introduced in 3.4 with commit 9cc05ad97c57 and fixed in 5.4.268 with commit d1fe1aede684
	Issue introduced in 3.4 with commit 9cc05ad97c57 and fixed in 5.10.209 with commit acd413da3e1f
	Issue introduced in 3.4 with commit 9cc05ad97c57 and fixed in 5.15.148 with commit 8fb12524c86b
	Issue introduced in 3.4 with commit 9cc05ad97c57 and fixed in 6.1.75 with commit 48dcfc42ce70
	Issue introduced in 3.4 with commit 9cc05ad97c57 and fixed in 6.6.14 with commit a34946ec3de8
	Issue introduced in 3.4 with commit 9cc05ad97c57 and fixed in 6.7.2 with commit f9b891a7e8fc
	Issue introduced in 3.4 with commit 9cc05ad97c57 and fixed in 6.8 with commit 86222a8fc16e

Please see https://www.kernel.org for a full list of currently supported
kernel versions by the kernel community.

Unaffected versions might change over time as fixes are backported to
older supported kernel versions.  The official CVE entry at
	https://cve.org/CVERecord/?id=CVE-2023-52685
will be updated if fixes are backported, please check that for the most
up to date information about this issue.


Affected files
==============

The file(s) affected by this issue are:
	fs/pstore/ram_core.c


Mitigation
==========

The Linux kernel CVE team recommends that you update to the latest
stable kernel version for this, and many other bugfixes.  Individual
changes are never tested alone, but rather are part of a larger kernel
release.  Cherry-picking individual commits is not recommended or
supported by the Linux kernel community at all.  If however, updating to
the latest release is impossible, the individual changes to resolve this
issue can be found at these commits:
	https://git.kernel.org/stable/c/3b333cded94fbe5ce30d699b316c4715151268ae
	https://git.kernel.org/stable/c/d1fe1aede684bd014714dacfdc75586a9ad38657
	https://git.kernel.org/stable/c/acd413da3e1f37582207cd6078a41d57c9011918
	https://git.kernel.org/stable/c/8fb12524c86bdd542a54857d5d076b1b6778c78c
	https://git.kernel.org/stable/c/48dcfc42ce705b652c0619cb99846afc43029de9
	https://git.kernel.org/stable/c/a34946ec3de88a16cc3a87fdab50aad06255a22b
	https://git.kernel.org/stable/c/f9b891a7e8fcf83901f8507241e23e7420103b61
	https://git.kernel.org/stable/c/86222a8fc16ec517de8da2604d904c9df3a08e5d

Re: CVE-2023-52685: pstore: ram_core: fix possible overflow in persistent_ram_init_ecc()

[Gabriel Krisman Bertazi]

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> Description
> ===========
>
> In the Linux kernel, the following vulnerability has been resolved:
>
> pstore: ram_core: fix possible overflow in persistent_ram_init_ecc()
>
> In persistent_ram_init_ecc(), on 64-bit arches DIV_ROUND_UP() will return
> 64-bit value since persistent_ram_zone::buffer_size has type size_t which
> is derived from the 64-bit *unsigned long*, while the ecc_blocks variable
> this value gets assigned to has (always 32-bit) *int* type.  Even if that
> value fits into *int* type, an overflow is still possible when calculating
> the size_t typed ecc_total variable further below since there's no cast to
> any 64-bit type before multiplication.  Declaring the ecc_blocks variable
> as *size_t* should fix this mess...
>
> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> analysis tool.

Hi Greg,

[Cc'ing Kees, who is listed as the pstore maintainer]

I want to dispute this CVE.  The overflow is in the module
initialization path, and can only happen at boot time or if the module
is loaded with specific parameters or due to specific acpi/device tree
data.  Either way, it would require root privileges to trigger.

-- 
Gabriel Krisman Bertazi

Re: CVE-2023-52685: pstore: ram_core: fix possible overflow in persistent_ram_init_ecc()

[Greg Kroah-Hartman]

On Mon, May 27, 2024 at 08:32:54PM -0400, Gabriel Krisman Bertazi wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> 
> > Description
> > ===========
> >
> > In the Linux kernel, the following vulnerability has been resolved:
> >
> > pstore: ram_core: fix possible overflow in persistent_ram_init_ecc()
> >
> > In persistent_ram_init_ecc(), on 64-bit arches DIV_ROUND_UP() will return
> > 64-bit value since persistent_ram_zone::buffer_size has type size_t which
> > is derived from the 64-bit *unsigned long*, while the ecc_blocks variable
> > this value gets assigned to has (always 32-bit) *int* type.  Even if that
> > value fits into *int* type, an overflow is still possible when calculating
> > the size_t typed ecc_total variable further below since there's no cast to
> > any 64-bit type before multiplication.  Declaring the ecc_blocks variable
> > as *size_t* should fix this mess...
> >
> > Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> > analysis tool.
> 
> Hi Greg,
> 
> [Cc'ing Kees, who is listed as the pstore maintainer]
> 
> I want to dispute this CVE.  The overflow is in the module
> initialization path, and can only happen at boot time or if the module
> is loaded with specific parameters or due to specific acpi/device tree
> data.  Either way, it would require root privileges to trigger.

Normally root privileges isn't the issue, as many containers allow root
to do things (including loading modules, crazy systems...)

Anyway, I'll defer to Kees as to if this should be revoked or not.

thanks,

gre gk-h

> 
> -- 
> Gabriel Krisman Bertazi

Re: CVE-2023-52685: pstore: ram_core: fix possible overflow in persistent_ram_init_ecc()

[Kees Cook]

On Tue, May 28, 2024 at 09:01:13PM +0200, Greg Kroah-Hartman wrote:
> On Mon, May 27, 2024 at 08:32:54PM -0400, Gabriel Krisman Bertazi wrote:
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > 
> > > Description
> > > ===========
> > >
> > > In the Linux kernel, the following vulnerability has been resolved:
> > >
> > > pstore: ram_core: fix possible overflow in persistent_ram_init_ecc()
> > >
> > > In persistent_ram_init_ecc(), on 64-bit arches DIV_ROUND_UP() will return
> > > 64-bit value since persistent_ram_zone::buffer_size has type size_t which
> > > is derived from the 64-bit *unsigned long*, while the ecc_blocks variable
> > > this value gets assigned to has (always 32-bit) *int* type.  Even if that
> > > value fits into *int* type, an overflow is still possible when calculating
> > > the size_t typed ecc_total variable further below since there's no cast to
> > > any 64-bit type before multiplication.  Declaring the ecc_blocks variable
> > > as *size_t* should fix this mess...
> > >
> > > Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> > > analysis tool.
> > 
> > Hi Greg,
> > 
> > [Cc'ing Kees, who is listed as the pstore maintainer]
> > 
> > I want to dispute this CVE.  The overflow is in the module
> > initialization path, and can only happen at boot time or if the module
> > is loaded with specific parameters or due to specific acpi/device tree
> > data.  Either way, it would require root privileges to trigger.
> 
> Normally root privileges isn't the issue, as many containers allow root
> to do things (including loading modules, crazy systems...)
> 
> Anyway, I'll defer to Kees as to if this should be revoked or not.

It's a module parameter or device tree value that is at most INT_MAX or
UINT_MAX respectively. Also, it is bounds checked against the buffer
itself:
        if (ecc_total >= prz->buffer_size) {

So even if it wrapped around and got "too small", there's no damage to
be had here.

The worst case is that the ramoops info goes missing because pstore
refuses to do anything with the bad value, but pstore can be disabled
way more easily than that, by design.

So, no, I don't think this is CVE worthy. I took the patch because it's
reasonable to try to get the math right and provide better error
reporting.

-- 
Kees Cook

Re: CVE-2023-52685: pstore: ram_core: fix possible overflow in persistent_ram_init_ecc()

[Greg Kroah-Hartman]

On Mon, Jun 17, 2024 at 02:17:49PM -0700, Kees Cook wrote:
> On Tue, May 28, 2024 at 09:01:13PM +0200, Greg Kroah-Hartman wrote:
> > On Mon, May 27, 2024 at 08:32:54PM -0400, Gabriel Krisman Bertazi wrote:
> > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > > 
> > > > Description
> > > > ===========
> > > >
> > > > In the Linux kernel, the following vulnerability has been resolved:
> > > >
> > > > pstore: ram_core: fix possible overflow in persistent_ram_init_ecc()
> > > >
> > > > In persistent_ram_init_ecc(), on 64-bit arches DIV_ROUND_UP() will return
> > > > 64-bit value since persistent_ram_zone::buffer_size has type size_t which
> > > > is derived from the 64-bit *unsigned long*, while the ecc_blocks variable
> > > > this value gets assigned to has (always 32-bit) *int* type.  Even if that
> > > > value fits into *int* type, an overflow is still possible when calculating
> > > > the size_t typed ecc_total variable further below since there's no cast to
> > > > any 64-bit type before multiplication.  Declaring the ecc_blocks variable
> > > > as *size_t* should fix this mess...
> > > >
> > > > Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> > > > analysis tool.
> > > 
> > > Hi Greg,
> > > 
> > > [Cc'ing Kees, who is listed as the pstore maintainer]
> > > 
> > > I want to dispute this CVE.  The overflow is in the module
> > > initialization path, and can only happen at boot time or if the module
> > > is loaded with specific parameters or due to specific acpi/device tree
> > > data.  Either way, it would require root privileges to trigger.
> > 
> > Normally root privileges isn't the issue, as many containers allow root
> > to do things (including loading modules, crazy systems...)
> > 
> > Anyway, I'll defer to Kees as to if this should be revoked or not.
> 
> It's a module parameter or device tree value that is at most INT_MAX or
> UINT_MAX respectively. Also, it is bounds checked against the buffer
> itself:
>         if (ecc_total >= prz->buffer_size) {
> 
> So even if it wrapped around and got "too small", there's no damage to
> be had here.
> 
> The worst case is that the ramoops info goes missing because pstore
> refuses to do anything with the bad value, but pstore can be disabled
> way more easily than that, by design.
> 
> So, no, I don't think this is CVE worthy. I took the patch because it's
> reasonable to try to get the math right and provide better error
> reporting.

Now rejected, thanks.

greg k-h