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: <20240816154244.0c6271a6@mordecai.tesarici.cz>
Date: Fri, 16 Aug 2024 15:42:44 +0200
From: Petr Tesarik <ptesarik@...e.com>
To: "Eric W. Biederman" <ebiederm@...ssion.com>
Cc: Sourabh Jain <sourabhjain@...ux.ibm.com>, Hari Bathini
 <hbathini@...ux.ibm.com>, Baoquan He <bhe@...hat.com>, Andrew Morton
 <akpm@...ux-foundation.org>, Eric DeVolder <eric_devolder@...oo.com>,
 kexec@...ts.infradead.org (open list:KEXEC), linux-kernel@...r.kernel.org
 (open list), stable@...nel.org
Subject: Re: [PATCH 1/1] kexec_file: fix elfcorehdr digest exclusion when
 CONFIG_CRASH_HOTPLUG=y

On Fri, 16 Aug 2024 07:54:52 -0500
"Eric W. Biederman" <ebiederm@...ssion.com> wrote:

> Petr Tesarik <petr.tesarik@...e.com> writes:
> 
> > From: Petr Tesarik <ptesarik@...e.com>
> >
> > Fix the condition to exclude the elfcorehdr segment from the SHA digest
> > calculation.
> >
> > The j iterator is an index into the output sha_regions[] array, not into
> > the input image->segment[] array. Once it reaches image->elfcorehdr_index,
> > all subsequent segments are excluded. Besides, if the purgatory segment
> > precedes the elfcorehdr segment, the elfcorehdr may be wrongly included in
> > the calculation.  
> 
> I would rather make CONFIG_CRASH_HOTPLUG depend on broken.
> 
> The hash is supposed to include everything we depend upon so when
> a borken machine corrupts something we can detect that corruption
> and not attempt to take a crash dump.
> 
> The elfcorehdr is definitely something that needs to be part of the
> hash.
> 
> So please go back to the drawing board and find a way to include the
> program header in the hash even with CONFIG_CRASH_HOTPLUG.

I'm not trying to argue with your opinion, but it seems you're
complaining to the wrong person. My present patch merely fixes an
obvious trivial mistake in commit f7cc804a9fd4 ("kexec: exclude
elfcorehdr from the segment digest") to exclude _only_ the elfcorehdr
segment from the hash (which was intended) and not any _other_ segments
(which was not intended but is what currently happens).

If you want to change the direction of kexec hotplug support, feel free
to revert commit f7cc804a9fd4 instead. That would also fix the bug and
make me happy.

Petr T

> Eric
> 
> 
> > Fixes: f7cc804a9fd4 ("kexec: exclude elfcorehdr from the segment digest")
> > Cc: stable@...nel.org
> > Signed-off-by: Petr Tesarik <ptesarik@...e.com>
> > ---
> >  kernel/kexec_file.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> > index 3d64290d24c9..3eedb8c226ad 100644
> > --- a/kernel/kexec_file.c
> > +++ b/kernel/kexec_file.c
> > @@ -752,7 +752,7 @@ static int kexec_calculate_store_digests(struct kimage *image)
> >  
> >  #ifdef CONFIG_CRASH_HOTPLUG
> >  		/* Exclude elfcorehdr segment to allow future changes via hotplug */
> > -		if (j == image->elfcorehdr_index)
> > +		if (i == image->elfcorehdr_index)
> >  			continue;
> >  #endif  


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ