| Message ID | 20230310171617.wqnqs42l2viwjsz5@archlinux (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Using MAP_SHARE_VALIDATE in mmap without fd | expand |
On 10.03.23 18:16, Nils Hartmann wrote: > Hey, > I have a rather simple question about the MAP_SHARED_VALIDATE flag in mmap. > When used without a file pointer, EINVAL is returned. Is there a reason for this? You mean, using it with shared anonymous memory? (MAP_SHARED|MAP_ANON) I assume you mean "file descriptor" not "file pointer". > I researched a bit but could not find anything. I attached a simple patch that adds MAP_SHARE_VALIDATE to the flags switch and checks for invalid flags. > The only reason MAP_SHARED_VALIDATE was introduced was due to MAP_SYNC, only required for DAX. DAX does not apply to shared anonymous memory. I guess nobody cared/cares. Question is if we want to update the implementation (there has to be a good reason IMHO) or simply update the man page, stating that MAP_SHARED_VALIDATE is not supported for MAP_ANON. > Signed-off-by: Nils Hartmann <nils1hartmann@gmail.com> > --- > mm/mmap.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/mm/mmap.c b/mm/mmap.c > index 740b54be3..fd7db51af 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -1196,6 +1196,7 @@ unsigned long do_mmap(struct file *file, unsigned long addr, > { > struct mm_struct *mm = current->mm; > vm_flags_t vm_flags; > + unsigned long flags_mask; > int pkey = 0; > > validate_mm(mm); > @@ -1266,14 +1267,14 @@ unsigned long do_mmap(struct file *file, unsigned long addr, > if (mlock_future_check(mm, vm_flags, len)) > return -EAGAIN; > > + flags_mask = LEGACY_MAP_MASK; > if (file) { > struct inode *inode = file_inode(file); > - unsigned long flags_mask; > > if (!file_mmap_ok(file, inode, pgoff, len)) > return -EOVERFLOW; > > - flags_mask = LEGACY_MAP_MASK | file->f_op->mmap_supported_flags; > + flags_mask |= file->f_op->mmap_supported_flags; > > switch (flags & MAP_TYPE) { > case MAP_SHARED: > @@ -1327,6 +1328,10 @@ unsigned long do_mmap(struct file *file, unsigned long addr, > } > } else { > switch (flags & MAP_TYPE) { > + case MAP_SHARED_VALIDATE: > + if (flags & ~flags_mask) > + return -EOPNOTSUPP; > + fallthrough; > case MAP_SHARED: > if (vm_flags & (VM_GROWSDOWN|VM_GROWSUP)) > return -EINVAL;
On Thu, Mar 16, 2023 at 04:28:21PM +0100, David Hildenbrand wrote: > On 10.03.23 18:16, Nils Hartmann wrote: > > Hey, > > When used without a file pointer, EINVAL is returned. Is there a reason for this? > > You mean, using it with shared anonymous memory? (MAP_SHARED|MAP_ANON) > I assume you mean "file descriptor" not "file pointer". Yup thats what I meant. > The only reason MAP_SHARED_VALIDATE was introduced was due to MAP_SYNC, only > required for DAX. DAX does not apply to shared anonymous memory. Yeah I heard about it being introduced with MAP_SYNC. But since the manpage from mmap specifically says: 'MAP_SHARED_VALIDATE provides the same behaviour as MAP_SHARED', I didn't think it would make a difference > I guess nobody cared/cares. > Question is if we want to update the implementation (there has to be a good > reason IMHO) or simply update the man page, stating that MAP_SHARED_VALIDATE > is not supported for MAP_ANON. The only reason I can come up with, is naming consistency. But it's really a non-issue und updating the man page is definitly the saner option. > -- > Thanks, > > David / dhildenb > Best Regards, Nils
diff --git a/mm/mmap.c b/mm/mmap.c index 740b54be3..fd7db51af 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1196,6 +1196,7 @@ unsigned long do_mmap(struct file *file, unsigned long addr, { struct mm_struct *mm = current->mm; vm_flags_t vm_flags; + unsigned long flags_mask; int pkey = 0; validate_mm(mm); @@ -1266,14 +1267,14 @@ unsigned long do_mmap(struct file *file, unsigned long addr, if (mlock_future_check(mm, vm_flags, len)) return -EAGAIN; + flags_mask = LEGACY_MAP_MASK; if (file) { struct inode *inode = file_inode(file); - unsigned long flags_mask; if (!file_mmap_ok(file, inode, pgoff, len)) return -EOVERFLOW; - flags_mask = LEGACY_MAP_MASK | file->f_op->mmap_supported_flags; + flags_mask |= file->f_op->mmap_supported_flags; switch (flags & MAP_TYPE) { case MAP_SHARED: @@ -1327,6 +1328,10 @@ unsigned long do_mmap(struct file *file, unsigned long addr, } } else { switch (flags & MAP_TYPE) { + case MAP_SHARED_VALIDATE: + if (flags & ~flags_mask) + return -EOPNOTSUPP; + fallthrough; case MAP_SHARED: if (vm_flags & (VM_GROWSDOWN|VM_GROWSUP)) return -EINVAL;
Hey, I have a rather simple question about the MAP_SHARED_VALIDATE flag in mmap. When used without a file pointer, EINVAL is returned. Is there a reason for this? I researched a bit but could not find anything. I attached a simple patch that adds MAP_SHARE_VALIDATE to the flags switch and checks for invalid flags. Signed-off-by: Nils Hartmann <nils1hartmann@gmail.com> --- mm/mmap.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)