netdev - Re: [Xen-devel] [PATCH v3] xen/grant-table: Avoid m2p

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: <alpine.DEB.2.02.1401211416420.21510@kaball.uk.xensource.com>
Date:	Tue, 21 Jan 2014 14:22:53 +0000
From:	Stefano Stabellini <stefano.stabellini@...citrix.com>
To:	David Vrabel <david.vrabel@...rix.com>
CC:	Stefano Stabellini <stefano.stabellini@...citrix.com>,
	Zoltan Kiss <zoltan.kiss@...rix.com>,
	<jonathan.davies@...rix.com>, <wei.liu2@...rix.com>,
	<ian.campbell@...rix.com>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <xen-devel@...ts.xenproject.org>
Subject: Re: [Xen-devel] [PATCH v3] xen/grant-table: Avoid m2p_override during
 mapping

On Tue, 21 Jan 2014, David Vrabel wrote:
> On 21/01/14 12:26, Stefano Stabellini wrote:
> > On Mon, 20 Jan 2014, Zoltan Kiss wrote:
> >
> >> -		ret = m2p_add_override(mfn, pages[i], kmap_ops ?
> >> -				       &kmap_ops[i] : NULL);
> >> +		if (m2p_override)
> >> +			ret = m2p_add_override(mfn, pages[i], kmap_ops ?
> >> +					       &kmap_ops[i] : NULL);
> >> +		else {
> >> +			unsigned long pfn = page_to_pfn(pages[i]);
> >> +			WARN_ON(PagePrivate(pages[i]));
> >> +			SetPagePrivate(pages[i]);
> >> +			set_page_private(pages[i], mfn);
> >> +			pages[i]->index = pfn_to_mfn(pfn);
> >> +			if (unlikely(!set_phys_to_machine(pfn, FOREIGN_FRAME(mfn))))
> >> +				return -ENOMEM;
> > 
> > What happens if the page is PageHighMem?
> > 
> > This looks like a subset of m2p_add_override, but it is missing some
> > relevant bits, like the PageHighMem check, or the p2m(m2p(mfn)) == mfn
> > check.  Maybe we can find a way to avoid duplicating the code.
> > We could split m2p_add_override in two functions or add yet another
> > parameter to it.
> 
> The PageHighMem() check isn't relevant as we're not mapping anything
> here.  Also, a page for a kernel grant mapping only cannot be highmem.
> 
> The check for a local mfn and the additional set_phys_to_machine() is
> only necessary if something tries an mfn_to_pfn() on the local mfn.  We
> can only omit adding an m2p override if we know thing will try
> mfn_to_pfn(), therefore the check and set_phys_to_machine() is unnecessary.

OK, you convinced me that the two checks are superfluous for this case.

Can we still avoid the code duplication by removing the corresponding
code from m2p_add_override and m2p_remove_override and doing the
set_page_private thing uniquely in grant-table.c?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html