This is a fix for a regression in 32 bit kernels caused by an invalid check for pgoff overflow in hugetlbfs mmap setup. The check incorrectly specified that the size of a loff_t was the same as the size of a long. The regression prevents mapping hugetlbfs files at offset greater than 4GB on 32 bit kernels.
Fix the check by using sizeof(loff_t) to get size. In addition, make sure pgoff + length can be represented by a signed long huge page offset. This check is only necessary on 32 bit kernels.
Fixes: 63489f8e8211 ("hugetlbfs: check for pgoff value overflow") Cc: stable@vger.kernel.org Reported-by: Dan Rue dan.rue@linaro.org Signed-off-by: Mike Kravetz mike.kravetz@oracle.com --- fs/hugetlbfs/inode.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c index b9a254dcc0e7..8450a1d75dfa 100644 --- a/fs/hugetlbfs/inode.c +++ b/fs/hugetlbfs/inode.c @@ -116,7 +116,8 @@ static void huge_pagevec_release(struct pagevec *pvec) * bit into account. */ #define PGOFF_LOFFT_MAX \ - (((1UL << (PAGE_SHIFT + 1)) - 1) << (BITS_PER_LONG - (PAGE_SHIFT + 1))) + (((1UL << (PAGE_SHIFT + 1)) - 1) << \ + ((sizeof(loff_t) * BITS_PER_BYTE) - (PAGE_SHIFT + 1)))
static int hugetlbfs_file_mmap(struct file *file, struct vm_area_struct *vma) { @@ -138,21 +139,32 @@ static int hugetlbfs_file_mmap(struct file *file, struct vm_area_struct *vma)
/* * page based offset in vm_pgoff could be sufficiently large to - * overflow a (l)off_t when converted to byte offset. + * overflow a loff_t when converted to byte offset. */ - if (vma->vm_pgoff & PGOFF_LOFFT_MAX) + if ((loff_t)vma->vm_pgoff & (loff_t)PGOFF_LOFFT_MAX) return -EINVAL;
- /* must be huge page aligned */ + /* vm_pgoff must be huge page aligned */ if (vma->vm_pgoff & (~huge_page_mask(h) >> PAGE_SHIFT)) return -EINVAL;
+ /* + * Compute file offset of the end of this mapping + */ vma_len = (loff_t)(vma->vm_end - vma->vm_start); len = vma_len + ((loff_t)vma->vm_pgoff << PAGE_SHIFT); - /* check for overflow */ + + /* Check to ensure this did not overflow loff_t */ if (len < vma_len) return -EINVAL;
+ /* + * On 32 bit systems, this check is necessary to ensure the last page + * of mapping can be represented as a signed long huge page index. + */ + if ((len >> huge_page_shift(h)) > LONG_MAX) + return -EINVAL; + inode_lock(inode); file_accessed(file);
Hi Mike,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master] [also build test WARNING on v4.16-rc7 next-20180329] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Mike-Kravetz/hugetlbfs-fix-bug-in-p... config: i386-randconfig-x000-201812 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386
All warnings (new ones prefixed by >>):
fs/hugetlbfs/inode.c: In function 'hugetlbfs_file_mmap':
fs/hugetlbfs/inode.c:119:35: warning: left shift count >= width of type [-Wshift-count-overflow]
(((1UL << (PAGE_SHIFT + 1)) - 1) << \ ^ fs/hugetlbfs/inode.c:144:38: note: in expansion of macro 'PGOFF_LOFFT_MAX' if ((loff_t)vma->vm_pgoff & (loff_t)PGOFF_LOFFT_MAX) ^~~~~~~~~~~~~~~
vim +119 fs/hugetlbfs/inode.c
110 111 /* 112 * Mask used when checking the page offset value passed in via system 113 * calls. This value will be converted to a loff_t which is signed. 114 * Therefore, we want to check the upper PAGE_SHIFT + 1 bits of the 115 * value. The extra bit (- 1 in the shift value) is to take the sign 116 * bit into account. 117 */ 118 #define PGOFF_LOFFT_MAX \
119 (((1UL << (PAGE_SHIFT + 1)) - 1) << \
120 ((sizeof(loff_t) * BITS_PER_BYTE) - (PAGE_SHIFT + 1))) 121
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
linux-stable-mirror@lists.linaro.org