linux-kernel - Re: [discuss] Re: Please pull x86-64 bug fixes

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 5 Oct 2006 18:29:03 -0700 (PDT)
From:	Linus Torvalds <torvalds@...l.org>
To:	Jeff Garzik <jeff@...zik.org>
cc:	Andi Kleen <ak@...e.de>, discuss@...-64.org,
	linux-kernel@...r.kernel.org
Subject: Re: [discuss] Re: Please pull x86-64 bug fixes



On Thu, 5 Oct 2006, Jeff Garzik wrote:

> Linus Torvalds wrote:
> > (And we should probably have the "pci=mmiocfg" kernel command line entry
> > that forces MMIOCFG regardless of any e820 issues, even for normal
> > accesses).
> 
> Something like this?

Yes, except I look at the resulting messy conditional:

        if ((type == 1) && (!(pci_probe & PCI_NO_CHECKS)) &&
                !e820_all_mapped(pci_mmcfg_config[0].base_address,
                        pci_mmcfg_config[0].base_address + MMCONFIG_APER_MIN,
                        E820_RESERVED)) {

and it's totally unreadable, so how about making this a helper inline 
function like

	static inline int validate_mmcfg(int type, unsigned long address)
	{
		/*
		 * If type 1 accesses don't work, assume we run on a
		 * Mac and always use MCFG
		 */
		if (type != 1)
			return 1;

		/*
		 * If the user asked us to not check the MMIOCFG base 
		 * address, just trust it.
		 */
		if (pci_probe & PCI_NO_CHECKS)
			return 1;

		/*
		 * Otherwise require that it's marked reserved in 
		 * the e820 tables
		 */
		return e820_all_mapped(address,
			address + MMCONFIG_APER_MIN,
			E820_RESERVED);
	}

and then just saying

	if (!validate_mmcfg(type, pci_mmcfg_config[0].base_address)) {
		.. complain and exit ..

which just seems a hell of a lot prettier to me.

It also means that _if_ we ever figure out other ways to double-check the 
address automatically (or we have some automatic way of saying "that can't 
be valid"), we can do so in that "validate" function, without adding more 
and more horrible code.

What say you? Let's try to keep the code readable (and preferably make it 
more so..)

			Linus
-
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/