On Sat 16-09-17 20:44:14, Dan Williams wrote:

On Tue, Aug 15, 2017 at 5:27 AM, Jan Kara [off-list ref] wrote:

quoted

On Mon 14-08-17 23:12:16, Dan Williams wrote:

quoted

The mmap syscall suffers from the ABI anti-pattern of not validating
unknown flags. However, proposals like MAP_SYNC and MAP_DIRECT need a
mechanism to define new behavior that is known to fail on older kernels
without the feature. Use the fact that specifying MAP_SHARED and
MAP_PRIVATE at the same time is invalid as a cute hack to allow a new
set of validated flags to be introduced.

This also introduces the ->fmmap() file operation that is ->mmap() plus
flags. Each ->fmmap() implementation must fail requests when a locally
unsupported flag is specified.

...

quoted

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 1104e5df39ef..bbe755d0caee 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h

@@ -1674,6 +1674,7 @@ struct file_operations {
      long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
      long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
      int (*mmap) (struct file *, struct vm_area_struct *);
+     int (*fmmap) (struct file *, struct vm_area_struct *, unsigned long);
      int (*open) (struct inode *, struct file *);
      int (*flush) (struct file *, fl_owner_t id);
      int (*release) (struct inode *, struct file *);

@@ -1748,6 +1749,12 @@ static inline int call_mmap(struct file *file, struct vm_area_struct *vma)
      return file->f_op->mmap(file, vma);
 }

+static inline int call_fmmap(struct file *file, struct vm_area_struct *vma,
+             unsigned long flags)
+{
+     return file->f_op->fmmap(file, vma, flags);
+}
+

Hum, I dislike a new file op for this when the only problem with ->mmap is
that it misses 'flags' argument. I understand there are lots of ->mmap
implementations out there and modifying prototype of them all is painful
but is it so bad? Coccinelle patch for this should be rather easy...

So it wasn't all that easy, and Linus declined to take it. I think we
should add a new ->mmap_validate() file operation and save the
tree-wide cleanup until later.

Well, we don't even strictly need the flags passed to ->mmap callback if we
are willing to use VMA flags. I want to use it for MAP_SYNC anyway... So
bumping vma->flags to u64 and using a flag is also an option (and frankly
I'd personally just go for that).

								Honza
-- 
Jan Kara [off-list ref]
SUSE Labs, CR