linux-kernel - Re: [PATCH 13/13] kdb: Add mdi, mdiW / mdiWcN commands to show iomapped memory

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]

Date: Tue, 18 Jun 2024 12:33:05 -0700
From: Doug Anderson <dianders@...omium.org>
To: Daniel Thompson <daniel.thompson@...aro.org>
Cc: kgdb-bugreport@...ts.sourceforge.net, 
	Christophe JAILLET <christophe.jaillet@...adoo.fr>, Jason Wessel <jason.wessel@...driver.com>, 
	Thorsten Blum <thorsten.blum@...lux.com>, Yuran Pereira <yuran.pereira@...mail.com>, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 13/13] kdb: Add mdi, mdiW / mdiWcN commands to show
 iomapped memory

Hi,

On Tue, Jun 18, 2024 at 8:59 AM Daniel Thompson
<daniel.thompson@...aro.org> wrote:
>
> On Mon, Jun 17, 2024 at 05:34:47PM -0700, Douglas Anderson wrote:
> > Add commands that are like the other "md" commands but that allow you
> > to read memory that's in the IO space.
> >
> > Signed-off-by: Douglas Anderson <dianders@...omium.org>
>
> Sorry to be the bearer of bad news but...
>
>
> > ---
> > <snip>
> > +/*
> > + * kdb_getioword
> > + * Inputs:
> > + *   word    Pointer to the word to receive the result.
> > + *   addr    Address of the area to copy.
> > + *   size    Size of the area.
> > + * Returns:
> > + *   0 for success, < 0 for error.
> > + */
> > +int kdb_getioword(unsigned long *word, unsigned long addr, size_t size)
> > +{
> > +     void __iomem *mapped = ioremap(addr, size);
>
> ioremap() is a might_sleep() function. It's unsafe to call it from the
> debug trap handler.

Hmmmm. Do you have a pointer to documentation or code showing that
it's a might_sleep() function. I was worried about this initially but
I couldn't find any documentation or code indicating it to be so. I
also got no warnings when I ran my prototype code and then the code
worked fine, so I assumed that it must somehow work. Sigh...

Looking more closely, maybe this is:

ioremap() -> ioremap_prot() -> generic_ioremap_prot() ->
__get_vm_area_caller() -> __get_vm_area_node() -> __get_vm_area_node()

...and that has a kernel allocation with GFP_KERNEL?

I guess it also then calls alloc_vmap_area()  which has might_sleep()...

I'll have to track down why no warnings triggered...

> I'm afraid I don't know a safe alternative either. Machinary such as
> kmap_atomic() needs a page and iomem won't have one.

Ugh. It would be nice to come up with something since it's not
uncommon to need to look at the state of hardware registers when a
crash happens. In the past I've managed to get into gdb, track down a
global variable where someone has already mapped the memory, and then
use gdb to look at the memory. It's always a big pain, though...

...even if I could just look up the virtual address where someone else
had already mapped it that might be enough. At least I wouldn't need
to go track down the globals myself...

...anyway, I guess I'll ponder on it and poke if I have time...

-Doug