linux-kernel - Re: [PATCH 2/2] crash_core: fix out-of-bounds access check in crash_exclude_mem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231219020213.33197-1-ytcoode@gmail.com>
Date: Tue, 19 Dec 2023 10:02:13 +0800
From: Yuntao Wang <ytcoode@...il.com>
To: akpm@...ux-foundation.org
Cc: bhe@...hat.com,
	bp@...en8.de,
	dave.hansen@...ux.intel.com,
	dyoung@...hat.com,
	hbathini@...ux.ibm.com,
	hpa@...or.com,
	kexec@...ts.infradead.org,
	linux-kernel@...r.kernel.org,
	mingo@...hat.com,
	seanjc@...gle.com,
	tglx@...utronix.de,
	tiwai@...e.de,
	vgoyal@...hat.com,
	x86@...nel.org,
	ytcoode@...il.com
Subject: Re: [PATCH 2/2] crash_core: fix out-of-bounds access check in crash_exclude_mem_range()

On Mon, 18 Dec 2023 09:29:02 -0800, Andrew Morton <akpm@...ux-foundation.org> wrote:

> On Mon, 18 Dec 2023 16:19:15 +0800 Yuntao Wang <ytcoode@...il.com> wrote:
> 
> > mem->nr_ranges represents the current number of elements stored in
> > the mem->ranges array, and mem->max_nr_ranges represents the maximum number
> > of elements that the mem->ranges array can hold. Therefore, the correct
> > array out-of-bounds check should be mem->nr_ranges >= mem->max_nr_ranges.
> > 
> 
> This does not apply after your own "crash_core: fix and simplify the
> logic of crash_exclude_mem_range()".  What should be done?

Hi Andrew,

I actually prefer the "crash_core: fix and simplify the logic of
crash_exclude_mem_range()" patch as it makes the final code more concise and
clear, and less prone to errors.

The current code is too strange, I guess no one can understand why there is
a break in the for loop when they read this code for the first time.

Moreover, I think the current code is too fragile, it relies on callers using
this function correctly to ensure its correctness, rather than being able to
guarantee the correctness on its own. I even feel that this function is very
likely to have bugs again as the code evolves.

However, Baoquan also has his own considerations, he suggests keeping the code
as it is.

The link below is our detailed discussion on this issue:

https://lore.kernel.org/lkml/20231214163842.129139-3-ytcoode@gmail.com/t/#mfd78a97e16251bcb190b0957a0b6cb4b0a096b54

The final decision on whether to apply that patch is up to you and Baoquan, if
you choose to apply that patch, this patch can be ignored. But if you decide not
to apply that patch, then this patch must be applied, as it fixes a bug in the
crash_exclude_mem_range() function.

Sincerely,
Yuntao