Re: [PATCH v6 1/5] vfs: add flags parameter to ->mmap() in 'struct file_operations'

[Dan Williams]

On Thu, Aug 24, 2017 at 9:58 AM, Christoph Hellwig  wrote:
> On Wed, Aug 23, 2017 at 04:48:40PM -0700, Dan Williams wrote:
>> We are running running short of vma->vm_flags. We can avoid needing a
>> new VM_* flag in some cases if the original @flags submitted to mmap(2)
>> is made available to the ->mmap() 'struct file_operations'
>> implementation. For example, the proposed addition of MAP_DIRECT can be
>> implemented without taking up a new vm_flags bit. Another motivation to
>> avoid vm_flags is that they appear in /proc/$pid/smaps, and we have seen
>> software that tries to dangerously (TOCTOU) read smaps to infer the
>> behavior of a virtual address range.
>>
>> This conversion was performed by the following semantic patch. There
>> were a few manual edits for oddities like proc_reg_mmap.
>>
>> Thanks to Julia for helping me with coccinelle iteration to cover cases
>> where the mmap routine is defined in a separate file from the 'struct
>> file_operations' instance that consumes it.
>
> How are we going to check that an instance actually supports any
> of those flags?

In patch 3 I validate the flags by introducing an
"mmap_supported_mask" field to 'struct file_operations'. It will be
zero by default for almost all implementations and zero means "support
the legacy mmap flags".
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v6 1/5] vfs: add flags parameter to ->mmap() in 'struct file_operations'

[Christoph Hellwig]

On Wed, Aug 23, 2017 at 04:48:40PM -0700, Dan Williams wrote:
> We are running running short of vma->vm_flags. We can avoid needing a
> new VM_* flag in some cases if the original @flags submitted to mmap(2)
> is made available to the ->mmap() 'struct file_operations'
> implementation. For example, the proposed addition of MAP_DIRECT can be
> implemented without taking up a new vm_flags bit. Another motivation to
> avoid vm_flags is that they appear in /proc/$pid/smaps, and we have seen
> software that tries to dangerously (TOCTOU) read smaps to infer the
> behavior of a virtual address range.
> 
> This conversion was performed by the following semantic patch. There
> were a few manual edits for oddities like proc_reg_mmap.
> 
> Thanks to Julia for helping me with coccinelle iteration to cover cases
> where the mmap routine is defined in a separate file from the 'struct
> file_operations' instance that consumes it.

How are we going to check that an instance actually supports any
of those flags?
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v6 1/5] vfs: add flags parameter to ->mmap() in 'struct file_operations'

[Dan Williams]

We are running running short of vma->vm_flags. We can avoid needing a
new VM_* flag in some cases if the original @flags submitted to mmap(2)
is made available to the ->mmap() 'struct file_operations'
implementation. For example, the proposed addition of MAP_DIRECT can be
implemented without taking up a new vm_flags bit. Another motivation to
avoid vm_flags is that they appear in /proc/$pid/smaps, and we have seen
software that tries to dangerously (TOCTOU) read smaps to infer the
behavior of a virtual address range.

This conversion was performed by the following semantic patch. There
were a few manual edits for oddities like proc_reg_mmap.

Thanks to Julia for helping me with coccinelle iteration to cover cases
where the mmap routine is defined in a separate file from the 'struct
file_operations' instance that consumes it.

// Usage:
// $ spatch mmap.cocci --no-includes --include-headers --dir .
//  --in-place ./ -j $num_cpus --very-quiet

virtual after_start

@initialize:ocaml@
@@

let tbl = Hashtbl.create(100)

let add_if_not_present fn =
  if not(Hashtbl.mem tbl fn) then Hashtbl.add tbl fn ()

@ a @
identifier fn;
identifier ops;
@@

struct file_operations ops = { ..., .mmap = fn, ...};

@script:ocaml@
fn << a.fn;
@@

add_if_not_present fn

@finalize:ocaml depends on !after_start@
tbls << merge.tbl;
@@

List.iter (fun t -> Hashtbl.iter (fun f _ -> add_if_not_present f) t) tbls;
Hashtbl.iter
(fun f _ ->
  let it = new iteration() in
  it#add_virtual_rule After_start;
  it#add_virtual_identifier Fn f;
  it#register())
tbl

@depends on after_start@
identifier virtual.fn;
identifier x, y;
@@

int fn(struct file *x,
struct vm_area_struct *y
-   )
+   , unsigned long map_flags)
{
...
}

@depends on after_start@
identifier virtual.fn;
identifier x, y;
@@

int fn(struct file *x,
struct vm_area_struct *y
-   );
+   , unsigned long map_flags);

@depends on after_start@
identifier virtual.fn;


@@

int fn(struct file *,
struct vm_area_struct *
-   );
+   , unsigned long);

Cc: Takashi Iwai 
Cc: David Airlie 
Cc: 
Cc: Daniel Vetter 
Signed-off-by: Julia Lawall 
Suggested-by: Jan Kara 
Cc: Andrew Morton 
Signed-off-by: Dan Williams 
---
 arch/arc/kernel/arc_hostlink.c |3 ++-
 arch/mips/kernel/vdso.c|2 +-
 arch/powerpc/kernel/proc_powerpc.c |3 ++-
 arch/powerpc/kvm/book3s_64_vio.c   |3 ++-
 arch/powerpc/platforms/cell/spufs/file.c   |   21 +---
 arch/powerpc/platforms/powernv/opal-prd.c  |3 ++-
 arch/um/drivers/mmapper_kern.c |3 ++-
 drivers/android/binder.c   |3 ++-
 drivers/char/agp/frontend.c|3 ++-
 drivers/char/bsr.c |3 ++-
 drivers/char/hpet.c|6 --
 drivers/char/mbcs.c|3 ++-
 drivers/char/mbcs.h|3 ++-
 drivers/char/mem.c |   11 +++---
 drivers/char/mspec.c   |9 ++---
 drivers/char/uv_mmtimer.c  |6 --
 drivers/dax/device.c   |3 ++-
 drivers/dma-buf/dma-buf.c  |4 +++-
 drivers/firewire/core-cdev.c   |3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h|3 ++-
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c   |5 +++--
 drivers/gpu/drm/arc/arcpgu_drv.c   |5 +++--
 drivers/gpu/drm/ast/ast_drv.h  |3 ++-
 drivers/gpu/drm/ast/ast_ttm.c  |3 ++-
 drivers/gpu/drm/bochs/bochs.h  |3 ++-
 drivers/gpu/drm/bochs/bochs_mm.c   |3 ++-
 drivers/gpu/drm/cirrus/cirrus_drv.h|3 ++-
 drivers/gpu/drm/cirrus/cirrus_ttm.c|3 ++-
 drivers/gpu/drm/drm_gem.c  |3 ++-
 drivers/gpu/drm/drm_gem_cma_helper.c   |6 --
 drivers/gpu/drm/drm_vm.c   |3 ++-
 drivers/gpu/drm/etnaviv/etnaviv_drv.h  |3 ++-
 drivers/gpu/drm/etnaviv/etnaviv_gem.c  |5 +++--
 drivers/gpu/drm/exynos/exynos_drm_gem.c|5 +++--
 drivers/gpu/drm/exynos/exynos_drm_gem.h|3 ++-
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h|3 ++-
 drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c|3 ++-
 drivers/gpu/drm/i810/i810_dma.c|3 ++-
 drivers/gpu/drm/i915/i915_gem_dmabuf.c |2 +-