Skip to contentSkip to conversation

hugetlbfs private mappings.

PATCH In review Oleg Nesterov 2 participants 4 messages Jul 11, 2004 – Jul 12, 2004 HugeTLB

lore
Share & tools
lore

Oleg Nesterov

author
#0 lore raw

PATCH 1/1 hugetlbfs private mappings.

Hello.

Hugetlbfs silently coerce private mappings of hugetlb files into shared ones. So private writable mapping has MAP_SHARED semantics. I think, such mappings should be disallowed.

First, such behavior allows open hugetlbfs file O_RDONLY, and overwrite it via mmap(PROT_READ|PROT_WRITE, MAP_PRIVATE), so it is security bug.

Second, private writable mmap() should fail just because kernel does not support this.

I beleive, it is ok to allow private readonly hugetlb mappings, sys_mprotect() does not work with hugetlb vmas.

There is another problem. Hugetlb mapping is always prefaulted, pages allocated at mmap() time. So even readonly mapping allows to enlarge the size of the hugetlbfs file, and steal huge pages without appropriative permissions.

Patch on top of vm_pgoff fixes, see
http://marc.theaimsgroup.com/?l=linux-kernel&m=108938233708584

Oleg.

Signed-off-by Oleg Nesterov <[email protected]>
fs/hugetlbfs/inode.c renamed from fs/hugetlbfs/inode.c.pgoff +14 -2
@@ -52,6 +52,9 @@ static int hugetlbfs_file_mmap(struct fi
 	loff_t len, vma_len;
 	int ret;
 
+	if ((vma->vm_flags & (VM_MAYSHARE | VM_WRITE)) == VM_WRITE)
+		return -EINVAL;
+
 	if (vma->vm_pgoff & (HPAGE_SIZE / PAGE_SIZE - 1))
 		return -EINVAL;
 

@@ -70,10 +73,19 @@ static int hugetlbfs_file_mmap(struct fi
 	file_accessed(file);
 	vma->vm_flags |= VM_HUGETLB | VM_RESERVED;
 	vma->vm_ops = &hugetlb_vm_ops;
+
+	ret = -ENOMEM;
+	len = vma_len + ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
+	if (!(vma->vm_flags & VM_WRITE) && len > inode->i_size)
+		goto out;
+
 	ret = hugetlb_prefault(mapping, vma);
-	len = vma_len +	((loff_t)vma->vm_pgoff << PAGE_SHIFT);
-	if (ret == 0 && inode->i_size < len)
+	if (ret)
+		goto out;
+
+	if (inode->i_size < len)
 		inode->i_size = len;
+out:
 	up(&inode->i_sem);
 
 	return ret;
#1 lore raw
Quoted 8 lines · Oleg Nesterov

Hugetlbfs silently coerce private mappings of hugetlb files into shared ones. So private writable mapping has MAP_SHARED semantics. I think, such mappings should be disallowed. First, such behavior allows open hugetlbfs file O_RDONLY, and overwrite it via mmap(PROT_READ|PROT_WRITE, MAP_PRIVATE), so it is security bug.
Second, private writable mmap() should fail just because kernel does not support this.

Quoted 8 lines

I beleive, it is ok to allow private readonly hugetlb mappings, sys_mprotect() does not work with hugetlb vmas.
There is another problem. Hugetlb mapping is always prefaulted, pages allocated at mmap() time. So even readonly mapping allows to enlarge the size of the hugetlbfs file, and steal huge pages without appropriative permissions.
Patch on top of vm_pgoff fixes, see
http://marc.theaimsgroup.com/?l=linux-kernel&m=108938233708584

This probably doesn't break anything worth caring about, but it may make people happier to just force MAP_SHARED on.

-- wli

author
#2 lore raw
Quoted 8 lines · William Lee Irwin III
Quoted 3 lines · Oleg Nesterov

Hugetlbfs silently coerce private mappings of hugetlb files into shared ones. So private writable mapping has MAP_SHARED semantics. I think, such mappings should be disallowed.

This probably doesn't break anything worth caring about, but it may make people happier to just force MAP_SHARED on.

Yes, it was my initial intent. Andrew Morton pointed out, that this could break existing applications.

Oleg.

#3 lore raw
Quoted 2 lines · William Lee Irwin III
Quoted 2 lines

This probably doesn't break anything worth caring about, but it may make people happier to just force MAP_SHARED on.

Quoted 2 lines · Oleg Nesterov

Yes, it was my initial intent. Andrew Morton pointed out, that this could break existing applications.

I see; I thought the concern went the other direction. All's well, then.

-- wli