linux-kernel - Re: [PATCH] x86: hold mm->page_table_lock while doing vmalloc

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:	Tue, 8 Feb 2011 00:20:45 +0100
From:	Andrea Arcangeli <aarcange@...hat.com>
To:	Jeremy Fitzhardinge <jeremy@...p.org>
Cc:	"H. Peter Anvin" <hpa@...or.com>,
	the arch/x86 maintainers <x86@...nel.org>,
	"Xen-devel@...ts.xensource.com" <Xen-devel@...ts.xensource.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Ian Campbell <Ian.Campbell@...rix.com>,
	Jan Beulich <JBeulich@...ell.com>,
	Larry Woodman <lwoodman@...hat.com>
Subject: Re: [PATCH] x86: hold mm->page_table_lock while doing vmalloc_sync

On Fri, Feb 04, 2011 at 01:27:33PM -0800, Jeremy Fitzhardinge wrote:
> No, I don't think there's any xen-specific code which calls mmdrop (at
> all, let alone in interrupt context).  Erm, but I'm not sure where it
> does.  I had a thinko that "schedule" would be one of those places, but
> calling that from interrupt context would cause much bigger problems :/
> > static void pgd_dtor(pgd_t *pgd)

I checked again and I don't see exactly where mmdrop or __mmdrop are
called from irq context.

> No.  I don't think I wrote that comment.  It possibly just some ancient
> lore that could have been correct at one point, or perhaps it never true.

I agree with that. But it'd be nice of more people could look into
that so we at least can remove the misleading comment.

Where else can the pgd_lock be taken from irq context? Can we fix the
deadlock with NR_CPUS < 4 with my patch? (with the ,flags removed from below?)


> 
> >>> @@ -247,7 +248,7 @@ void vmalloc_sync_all(void)
> >>>  			if (!ret)
> >>>  				break;
> >>>  		}
> >>> -		spin_unlock_irqrestore(&pgd_lock, flags);
> >>> +		spin_unlock(&pgd_lock, flags);
> >> Urp.  Did this compile?
> > Yes it builds
> 
> (spin_unlock() shouldn't take a "flags" arg.)
> 
> 
> > I'm not reposting a version that builds for 32bit x86 too until we
> > figure out the mmdrop thing...
> 
> Stick it in next and look for explosion reports?

I intended to correct that of course, I just meant it is no problem
for 64bit builds and that's why I didn't notice the build failure
before posting the patch. Clearly 32bit build would have failed ;).
--
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/