Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210727111446.119561-1-mlombard@redhat.com> <YQDznBi5NuTIKd+x@kernel.org>
In-Reply-To: <YQDznBi5NuTIKd+x@kernel.org>
From:   Maurizio Lombardi <mlombard@redhat.com>
Date:   Wed, 28 Jul 2021 11:39:25 +0200
Message-ID: <CAFL455=X7aCK=vSBuQZV39gR03mrnc1j+-YHWkWbVor4g89DkQ@mail.gmail.com>
Subject: Re: [PATCH V2] iscsi_ibft: fix crash due to KASLR physical memory remapping
To:     Mike Rapoport <rppt@kernel.org>
Cc:     bp@alien8.de, tglx@linutronix.de, x86@kernel.org,
        pjones@redhat.com, konrad@kernel.org,
        George Kennedy <george.kennedy@oracle.com>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Mike,

st 28. 7. 2021 v 8:05 odes=C3=ADlatel Mike Rapoport <rppt@kernel.org> napsa=
l:
>
> Hi,
>
> On Tue, Jul 27, 2021 at 01:14:46PM +0200, Maurizio Lombardi wrote:
> > Starting with commit a799c2bd29d19c565f37fa038b31a0a1d44d0e4d
> > memory reservations have been moved earlier during the boot process,
> > before the execution of the Kernel Address Space Layout Randomization c=
ode.
> >
> > setup_arch() calls the iscsi_ibft's find_ibft_region() function
> > to find and reserve the memory dedicated to the iBFT; this function
> > also saves a (virt) pointer to the iBFT table for later use.
>
>               ^ virtual
>
> > The problem is that if KALSR is active, the physical memory gets
> > remapped somewhere else in the virtual address space and the pointer is
> > no longer valid, this will cause a kernel panic when the iscsi driver t=
ries
> > to dereference it.
>
> Please drop "this patch" and use imperative language, e.g. "Fix this bug
> by..."
>
> > This patch fixes the bug by saving the address of the physical location
> > of the ibft; later the driver will use isa_bus_to_virt() to get
> > the correct virtual address.
> >
> > It will also simplify the code by renaming find_ibft_region()
> > to reserve_ibft_region() and removing all the wrappers.
> >
> > [   37.764225] iBFT detected.
>
> It is better to put the crash report close to the problem description as =
it
> actually shows how the issue is manifested.

Thanks for the review, I will submit a V3 soon

Maurizio