| Message ID | 20240705071150.84972-1-donettom@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fs/hugetlbfs/inode.c: Ensure generic_hugetlb_get_unmapped_area() returns higher address than mmap_min_addr | expand |
On Fri, Jul 05, 2024 at 02:11:50AM -0500, Donet Tom wrote: > diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c > index 412f295acebe..428fd2f0e4c4 100644 > --- a/fs/hugetlbfs/inode.c > +++ b/fs/hugetlbfs/inode.c > @@ -228,7 +228,7 @@ generic_hugetlb_get_unmapped_area(struct file *file, unsigned long addr, > > if (len & ~huge_page_mask(h)) > return -EINVAL; > - if (len > TASK_SIZE) > + if (len > mmap_end - mmap_min_addr) > return -ENOMEM; > > if (flags & MAP_FIXED) { > @@ -240,7 +240,7 @@ generic_hugetlb_get_unmapped_area(struct file *file, unsigned long addr, > if (addr) { > addr = ALIGN(addr, huge_page_size(h)); > vma = find_vma(mm, addr); > - if (mmap_end - len >= addr && > + if (mmap_end - len >= addr && addr >= mmap_min_addr && > (!vma || addr + len <= vm_start_gap(vma))) > return addr; > } There's more difference with generic_get_unmapped_area() than what you are fixing. I think we also need vm_end_gap() here. Hugetlb code duplication is annoying.
On 7/5/24 17:53, Kirill A . Shutemov wrote: > On Fri, Jul 05, 2024 at 02:11:50AM -0500, Donet Tom wrote: >> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c >> index 412f295acebe..428fd2f0e4c4 100644 >> --- a/fs/hugetlbfs/inode.c >> +++ b/fs/hugetlbfs/inode.c >> @@ -228,7 +228,7 @@ generic_hugetlb_get_unmapped_area(struct file *file, unsigned long addr, >> >> if (len & ~huge_page_mask(h)) >> return -EINVAL; >> - if (len > TASK_SIZE) >> + if (len > mmap_end - mmap_min_addr) >> return -ENOMEM; >> >> if (flags & MAP_FIXED) { >> @@ -240,7 +240,7 @@ generic_hugetlb_get_unmapped_area(struct file *file, unsigned long addr, >> if (addr) { >> addr = ALIGN(addr, huge_page_size(h)); >> vma = find_vma(mm, addr); >> - if (mmap_end - len >= addr && >> + if (mmap_end - len >= addr && addr >= mmap_min_addr && >> (!vma || addr + len <= vm_start_gap(vma))) >> return addr; >> } > There's more difference with generic_get_unmapped_area() than what you are > fixing. I think we also need vm_end_gap() here. Thank you for your comment. I will add this change and send V2. -Donet > > Hugetlb code duplication is annoying. >
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c index 412f295acebe..428fd2f0e4c4 100644 --- a/fs/hugetlbfs/inode.c +++ b/fs/hugetlbfs/inode.c @@ -228,7 +228,7 @@ generic_hugetlb_get_unmapped_area(struct file *file, unsigned long addr, if (len & ~huge_page_mask(h)) return -EINVAL; - if (len > TASK_SIZE) + if (len > mmap_end - mmap_min_addr) return -ENOMEM; if (flags & MAP_FIXED) { @@ -240,7 +240,7 @@ generic_hugetlb_get_unmapped_area(struct file *file, unsigned long addr, if (addr) { addr = ALIGN(addr, huge_page_size(h)); vma = find_vma(mm, addr); - if (mmap_end - len >= addr && + if (mmap_end - len >= addr && addr >= mmap_min_addr && (!vma || addr + len <= vm_start_gap(vma))) return addr; }
generic_hugetlb_get_unmapped_area() was returning an address less than mmap_min_addr if the mmap argument addr, after alignment, was less than mmap_min_addr, causing mmap to fail. This is because current generic_hugetlb_get_unmapped_area() code does not take into account mmap_min_addr. This patch ensures that generic_hugetlb_get_unmapped_area() always returns an address that is greater than mmap_min_addr. How to reproduce ================ #include <stdio.h> #include <stdlib.h> #include <sys/mman.h> #include <unistd.h> #define HUGEPAGE_SIZE (16 * 1024 * 1024) int main() { void *addr = mmap((void *)-1, HUGEPAGE_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0); if (addr == MAP_FAILED) { perror("mmap"); exit(EXIT_FAILURE); } snprintf((char *)addr, HUGEPAGE_SIZE, "Hello, Huge Pages!"); printf("%s\n", (char *)addr); if (munmap(addr, HUGEPAGE_SIZE) == -1) { perror("munmap"); exit(EXIT_FAILURE); } return 0; } Result without fix ================== # cat /proc/meminfo |grep -i HugePages_Free HugePages_Free: 20 # ./test mmap: Permission denied # Result with fix =============== # cat /proc/meminfo |grep -i HugePages_Free HugePages_Free: 20 # ./test Hello, Huge Pages! # Reported-by Pavithra Prakash <pavrampu@linux.vnet.ibm.com> Signed-off-by: Donet Tom <donettom@linux.ibm.com> --- fs/hugetlbfs/inode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)