From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 110010 invoked by alias); 16 Mar 2015 19:20:28 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 109998 invoked by uid 89); 16 Mar 2015 19:20:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-lb0-f178.google.com Received: from mail-lb0-f178.google.com (HELO mail-lb0-f178.google.com) (209.85.217.178) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 16 Mar 2015 19:20:26 +0000 Received: by lbbzq9 with SMTP id zq9so38096342lbb.0 for ; Mon, 16 Mar 2015 12:20:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=WlHr2l3IGRoxBK4l2iuojw/a4yS+Yqdaxuq/LDTN0v0=; b=jb6owQIFpYyPkhWHj8NsQBvg2XGxgUFxLMNz/muFqVfJzTwoe9UIdc+k8g2CYpZM9+ kYYhbWVbqHffuSJJCZylhQ+p1yXyW5F7+cfxcc6k9UJsYI87y3W9nWaxmKYnLm2cCwyR 69ODZaQ0kv14ADfYyryTs9USW+ZMyXCrK0LEsb67aXkUj8abb3VW7oavVJ9YTibtUFO1 WDewhLtepXHzv7ZMai+KC6NbEMIF9KCDzbcTMMD6md8h2DJG50h74MTFPeVafHGr/74B /VAsDnX3hurMHM5hEoUEtKemPu+juihDczgZNxOekbbnMApQtzMPXfXW3E32ilorurrV dKxw== X-Gm-Message-State: ALoCoQkGL/mYoz7p7NU8dk9LcI/9aHpxjuBcssfTcArD4MClGwH8HMxt4zNi67I/fAQYjCUbmy08 X-Received: by 10.112.64.2 with SMTP id k2mr57654948lbs.54.1426533623283; Mon, 16 Mar 2015 12:20:23 -0700 (PDT) MIME-Version: 1.0 Received: by 10.152.111.232 with HTTP; Mon, 16 Mar 2015 12:20:03 -0700 (PDT) In-Reply-To: <20150316190154.GA18472@redhat.com> References: <878ufc9kau.fsf@redhat.com> <20150305154827.GA9441@host1.jankratochvil.net> <87zj7r5fpz.fsf@redhat.com> <20150305205744.GA13165@host1.jankratochvil.net> <20150311200052.GA22654@redhat.com> <20150312143438.GA4338@redhat.com> <20150312165423.GA10073@redhat.com> <20150312174653.GA13086@redhat.com> <20150316190154.GA18472@redhat.com> From: Andy Lutomirski Date: Mon, 16 Mar 2015 19:20:00 -0000 Message-ID: Subject: Re: install_special_mapping && vm_pgoff (Was: vvar, gup && coredump) To: Oleg Nesterov Cc: Hugh Dickins , Linus Torvalds , Jan Kratochvil , Sergio Durigan Junior , GDB Patches , Pedro Alves , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2015-03/txt/msg00467.txt.bz2 [cc: linux-mm] On Mon, Mar 16, 2015 at 12:01 PM, Oleg Nesterov wrote: > On 03/12, Oleg Nesterov wrote: >> >> OTOH. We can probably add ->access() into special_mapping_vmops, this >> way __access_remote_vm() could work even if gup() fails ? > > So I tried to think how special_mapping_vmops->access() can work, it > needs to rely on ->vm_pgoff. > > But afaics this logic is just broken. Lets even forget about vvar vma > which uses remap_file_pages(). Lets look at "[vdso]" which uses the > "normal" pages. > > The comment in special_mapping_fault() says > > * special mappings have no vm_file, and in that case, the mm > * uses vm_pgoff internally. > > Yes. But afaics mm/ doesn't do this correctly. So > > * do not copy this code into drivers! > > looks like a good recommendation ;) > > I think that this logic is wrong even if ARRAY_SIZE(pages) == 1, but I am > not sure. But since vdso use 2 pages, it is trivial to show that this logic > is wrong. To verify, I changed show_map_vma() to expose pgoff even if !file, > but this test-case can show the problem too: > > #include > #include > #include > #include > #include > #include > > void *find_vdso_vaddr(void) > { > FILE *perl; > char buf[32] = {}; > > perl = popen("perl -e 'open STDIN,qq|/proc/@{[getppid]}/maps|;" > "/^(.*?)-.*vdso/ && print hex $1 while <>'", "r"); > fread(buf, sizeof(buf), 1, perl); > fclose(perl); > > return (void *)atol(buf); > } > > #define PAGE_SIZE 4096 > > int main(void) > { > void *vdso = find_vdso_vaddr(); > assert(vdso); > > // of course they should differ, and they do so far > printf("vdso pages differ: %d\n", > !!memcmp(vdso, vdso + PAGE_SIZE, PAGE_SIZE)); > > // split into 2 vma's > assert(mprotect(vdso, PAGE_SIZE, PROT_READ) == 0); > > // force another fault on the next check > assert(madvise(vdso, 2 * PAGE_SIZE, MADV_DONTNEED) == 0); I really hope this doesn't do anything (or fails) on the vvar page, which is a pfnmap. > > // now they no longer differ, the 2nd vm_pgoff is wrong > printf("vdso pages differ: %d\n", > !!memcmp(vdso, vdso + PAGE_SIZE, PAGE_SIZE)); > > return 0; > } > > output: > > vdso pages differ: 1 > vdso pages differ: 0 > > And not only "split_vma" is wrong, I think that "move_vma" is not right too. > Note this check in copy_vma(), > > /* > * If anonymous vma has not yet been faulted, update new pgoff > * to match new location, to increase its chance of merging. > */ > if (unlikely(!vma->vm_file && !vma->anon_vma)) { > pgoff = addr >> PAGE_SHIFT; > faulted_in_anon_vma = false; > } > > I can easily misread this code. But it doesn't look right too. If vdso was cow'ed > (breakpoint installed by gdb) and sys_nremap()'ed, then the new pgoff will be wrong > too after, say, MADV_DONTNEED. > > Or I am totally confused? Ick, you're probably right. For what it's worth, the vdso *seems* to be okay (on 64-bit only, and only if you don't poke at it too hard) if you mremap it in one piece. CRIU does that. What does the mm code do with vm_pgoff for vmas with no vm_file? I'm mystified. There's this comment: * The way we recognize COWed pages within VM_PFNMAP mappings is through the * rules set up by "remap_pfn_range()": the vma will have the VM_PFNMAP bit * set, and the vm_pgoff will point to the first PFN mapped: thus every special * mapping will always honor the rule * * pfn_of_page == vma->vm_pgoff + ((addr - vma->vm_start) >> PAGE_SHIFT) Is that referring to special mappings in the install_special_mapping sense or to something else. FWIW, the vdso ins't a VM_PFNMAP at all. --Andy