linux-kernel - Re: [PATCH] fbdev: suppress warning when assigning vga-save/restore base

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:	Mon, 5 Aug 2013 22:29:55 +0200
From:	Ondrej Zajicek <santiago@...reenet.org>
To:	David Miller <davem@...emloft.net>
Cc:	hpa@...or.com, dh.herrmann@...il.com, linux-fbdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, plagnioj@...osoft.com,
	tomi.valkeinen@...com, Ondrej Zary <linux@...nbow-software.org>
Subject: Re: [PATCH] fbdev: suppress warning when assigning
	vga-save/restore base

On Sun, Aug 04, 2013 at 06:51:46PM -0700, David Miller wrote:
> From: "H. Peter Anvin" <hpa@...or.com>
> Date: Sun, 04 Aug 2013 10:33:46 -0700
> 
> > Anyone who can dig backwards and summarize?  In other words:
> > 
> > Where in the current code do we stuff a physical address in a pointer,
> > or a virtual address into a non-pointer?
> 
> The VGA register accessors try to accomodate iomem and ioport
> accesses.
> 
> If they are given a non-NULL iomem pointer 'regbase' they use
> iomem accesses, otherwise they do direct ISA port poking.
> 
> And yes the drivers in question are making some brash assumptions.
> I suspect they should be using ioremap() or similar.

Well, these drivers were written without MMIO (port IO only with NULL
instead of 'vgabase' for VGA register accessors). They were converted in
patches 94c322c30bd14ae6cdd369cb4a1f94c5c3809ac9,
f8645933513c65ac55f23c63b2649097289795c6 and a few others (from David
Miller) to potentially use MMIO by using 'vgabase' instead of NULL:

 pcibios_bus_to_resource(dev, &vga_res, &bus_reg);
 par->state.vgabase = (void __iomem *) vga_res.start;

How this could even work? AFAIK these cards have to be explicitly programmed
to enable MMIO (which was not done in the patches). These patches claim that
it is for multi-domain PCI. I would guess that vgabase is NULL in common
configurations but if it is non-NULL, it probably wouldn't work, unless
there is some hardware magic that transparently converts MMIO (from CPU PoV)
to port IO (from card/PCI PoV).

Note that there are some later patches (86c0f043a737dadf034a4e6f29aefb074f4a1146)
from Ondrej Zary that independently enable MMIO, but they use par->mmio
field instead of par->state.vgabase .

-- 
Elen sila lumenn' omentielvo

Ondrej 'SanTiago' Zajicek (email: santiago@...reenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."
--
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/