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:	Tue, 10 Oct 2006 21:10:09 +0100 (BST)
From:	Hugh Dickins <hugh@...itas.com>
To:	"Chen, Kenneth W" <kenneth.w.chen@...el.com>
cc:	"'David Gibson'" <david@...son.dropbear.id.au>,
	Andrew Morton <akpm@...l.org>, linux-kernel@...r.kernel.org
Subject: RE: Hugepage regression

On Tue, 10 Oct 2006, Chen, Kenneth W wrote:
> Hugh Dickins wrote on Tuesday, October 10, 2006 12:18 PM
> 
> > But again, I protest
> > the "if (vma->vm_file)" in your unmap_hugepage_range - how would a
> > hugepage area ever have NULL vma->vm_file?
> 
> It's coming from do_mmap_pgoff(), file->f_op->mmap can fail with error
> code (e.g. not enough hugetlb page) and in the error recovery path, it
> nulls out vma->vm_file first before calls down to unmap_region().

I stand corrected: thanks.

> I asked that question before:

So you did, on Oct 2nd: sorry, that got lost amidst the overload in
my mailbox, I've just forwarded it to myself again, to check later
on what else I may have missed.  (I am aware that I've still not
scrutinized the patches you sent out a day or two later, now in -mm.)

> can we reverse that order (call unmap_region
> and then nulls out vma->vmfile and fput)?

I'm pretty sure we cannot: the ordering is quite intentional, that if
a driver ->mmap failed, then it'd be wrong to call down to driver in
the unmap_region (if a driver is nicely behaved, that unmap_region
shouldn't be unnecessary; but some do rely on us clearing ptes there).

Okay, last refuge of all who've made a fool of themselves:
may I ask you to add a comment in your unmap_hugepage_range,
pointing to how the do_mmap_pgoff error case NULLifies vm_file?

(Or else change hugetlbfs_file_mmap to set VM_HUGETLB only once it's
succeeded: but that smacks of me refusing to accept I was wrong.)

Hugh

> 
> unmap_and_free_vma:
>         if (correct_wcount)
>                 atomic_inc(&inode->i_writecount);
>         vma->vm_file = NULL;
>         fput(file);
> 
>         /* Undo any partial mapping done by a device driver. */
>         unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end);
-
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ