Re: [PATCH 4.15 000/105] 4.15.14-stable review

From: Mike Kravetz
Date: Wed Mar 28 2018 - 20:43:03 EST


On 03/28/2018 12:06 PM, Mike Kravetz wrote:
> On 03/28/2018 11:44 AM, Dan Rue wrote:
>> On Tue, Mar 27, 2018 at 06:26:40PM +0200, Greg Kroah-Hartman wrote:
>>> This is the start of the stable review cycle for the 4.15.14 release.
>>> There are 105 patches in this series, all will be posted as a response
>>> to this one. If anyone has any issues with these being applied, please
>>> let me know.
>>>
>>> Responses should be made by Thu Mar 29 16:27:29 UTC 2018.
>>> Anything received after that time might be too late.
>>
>> Results from Linaroâs test farm.
>> No regressions on arm64 and x86_64.
>>
>> There is a regression on arm32 in libhugetlbfs/truncate_above_4GB-2M-32
>> that also exists in 4.14 and mainline. We'll investigate the root cause
>> and report upstream in mainline. I suspect the cause is "hugetlbfs:
>> check for pgoff value overflow", but have not verified yet.
>
> I'll also take a look as this was a patch I introduced.


I do not have an arm32 system to test, but am fairly confident the commit
63489f8e8211 (hugetlbfs: check for pgoff value overflow) introduced this
regression. A new mask was added to check PGOFF for overflow of a lofft.

/*
* Mask used when checking the page offset value passed in via system
* calls. This value will be converted to a loff_t which is signed.
* Therefore, we want to check the upper PAGE_SHIFT + 1 bits of the
* value. The extra bit (- 1 in the shift value) is to take the sign
* bit into account.
*/
#define PGOFF_LOFFT_MAX \
(((1UL << (PAGE_SHIFT + 1)) - 1) << (BITS_PER_LONG - (PAGE_SHIFT + 1)))

This mask incorrectly used BITS_PER_LONG as the size of a loff_t. This is
true on 64 bit, but not 32 bit systems. As a result, mmap of hugetlbfs file
offsets near 4GB will fail. I suspect the mmap64 call in the test
libhugetlbfs/truncate_above_4GB was the source of the failure.

Can you try the following on arm32? I will try to set up a 32 bit
x86 system to test as well. But, it may take me a bit of time.

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);

--
Mike Kravetz