linux-kernel - Re: [PATCH 6 of 7] x86: use early_ioremap in __acpi_map

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: <20080908142619.GA10580@elte.hu>
Date:	Mon, 8 Sep 2008 16:26:19 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Jeremy Fitzhardinge <jeremy@...p.org>
Cc:	Andi Kleen <andi@...stfloor.org>, linux-kernel@...r.kernel.org,
	"H. Peter Anvin" <hpa@...or.com>,
	Xen Devel <xen-devel@...ts.xensource.com>
Subject: Re: [PATCH 6 of 7] x86: use early_ioremap in __acpi_map_table

* Jeremy Fitzhardinge <jeremy@...p.org> wrote:

> >> However, unlike early_ioremap(), __acpi_map_table() just maintains 
> >> a single mapping which gets replaced each call, and has no 
> >> corresponding unmap function.  Implement this by just removing the 
> >> previous mapping each time its called.  Unfortunately, this will 
> >> leave a stray mapping at the end.
> >
> > It would be better to just fix the ACPI code to unmap.
> 
> I was concerned that would cause lots of cross-arch churn, but of 
> course the only other relevant architecture is ia64.  I'll prep a 
> followup patch.

uhm, there's a nasty trap in that route: it can potentially cause a lot 
of breakage.

It's not robust to assume that the ACPI code is sane wrt. 
mapping/unmapping, because it currently simply doesnt rely on robust 
unmapping (in the linear range).

I tried it in the past and i found tons of crappy ACPI code all around 
that just never unmapped tables. Leaking ACPI maps are hard to find as 
well, and it can occur anytime during bootup.

As a general principle it might be worth fixing those places, and we've 
hardened up the early-ioremap code for leaks during the PAT rewrite, 
still please realize that it can become non-trivial and it might cause a 
lot of unhappy users.

So i'd suggest a different, more carful approach: keep the new code you 
wrote, but print a WARN()ing if prev_map is not unmapped yet when the 
next mapping is acquired. That way the ACPI code can be fixed gradually 
and without breaking existing functionality.

There's another complication: ACPI might rely on multiple mappings being 
present at once, so unmapping the previous one might not be safe. But it 
_should_ be fine most of the time as __acpi_map_table() is only used 
inearly init code - and we fixed most of these things in the PAT 
patchset in any case.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/