[PATCH] dma-buf: Deny copy-on-writes mmaps

[PATCH] dma-buf: Deny copy-on-writes mmaps

[Andi Shyti]

From: Chris Wilson <chris.p.wilson@linux.intel.com>

Enforce that an mmap of a dmabuf is always using MAP_SHARED so that all
access (both read and writes) using the device memory and not a local
copy-on-write page in system memory.

Signed-off-by: Chris Wilson <chris.p.wilson@linux.intel.com>
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
---
 drivers/dma-buf/dma-buf.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 21916bba77d5..1ec297241842 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -25,6 +25,7 @@
 #include <linux/poll.h>
 #include <linux/dma-resv.h>
 #include <linux/mm.h>
+#include <linux/mman.h>
 #include <linux/mount.h>
 #include <linux/pseudo_fs.h>
 
@@ -128,6 +129,19 @@ static struct file_system_type dma_buf_fs_type = {
 	.kill_sb = kill_anon_super,
 };
 
+static unsigned long
+dma_buf_get_unmapped_area(struct file *file,
+			  unsigned long addr,
+			  unsigned long len,
+			  unsigned long pgoff,
+			  unsigned long flags)
+{
+	if ((flags & MAP_TYPE) == MAP_PRIVATE)
+		return -EINVAL;
+
+	return current->mm->get_unmapped_area(file, addr, len, pgoff, flags);
+}
+
 static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
 {
 	struct dma_buf *dmabuf;
@@ -508,6 +522,7 @@ static void dma_buf_show_fdinfo(struct seq_file *m, struct file *file)
 
 static const struct file_operations dma_buf_fops = {
 	.release	= dma_buf_file_release,
+	.get_unmapped_area = dma_buf_get_unmapped_area,
 	.mmap		= dma_buf_mmap_internal,
 	.llseek		= dma_buf_llseek,
 	.poll		= dma_buf_poll,
-- 
2.40.1

Re: [PATCH] dma-buf: Deny copy-on-writes mmaps

[kernel test robot]

Hi Andi,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v6.6-rc4 next-20231003]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andi-Shyti/dma-buf-Deny-copy-on-writes-mmaps/20231004-070556
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20231003230332.513051-1-andi.shyti%40linux.intel.com
patch subject: [PATCH] dma-buf: Deny copy-on-writes mmaps
config: sh-allyesconfig (https://download.01.org/0day-ci/archive/20231004/202310041156.Bi2Vshvb-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231004/202310041156.Bi2Vshvb-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310041156.Bi2Vshvb-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/dma-buf/dma-buf.c: In function 'dma_buf_get_unmapped_area':
>> drivers/dma-buf/dma-buf.c:142:27: error: 'struct mm_struct' has no member named 'get_unmapped_area'
     142 |         return current->mm->get_unmapped_area(file, addr, len, pgoff, flags);
         |                           ^~
   drivers/dma-buf/dma-buf.c:143:1: error: control reaches end of non-void function [-Werror=return-type]
     143 | }
         | ^
   cc1: some warnings being treated as errors


vim +142 drivers/dma-buf/dma-buf.c

   131	
   132	static unsigned long
   133	dma_buf_get_unmapped_area(struct file *file,
   134				  unsigned long addr,
   135				  unsigned long len,
   136				  unsigned long pgoff,
   137				  unsigned long flags)
   138	{
   139		if ((flags & MAP_TYPE) == MAP_PRIVATE)
   140			return -EINVAL;
   141	
 > 142		return current->mm->get_unmapped_area(file, addr, len, pgoff, flags);
   143	}
   144	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] dma-buf: Deny copy-on-writes mmaps

[Christian König]

Am 04.10.23 um 01:03 schrieb Andi Shyti:
> From: Chris Wilson <chris.p.wilson@linux.intel.com>
>
> Enforce that an mmap of a dmabuf is always using MAP_SHARED so that all
> access (both read and writes) using the device memory and not a local
> copy-on-write page in system memory.

As much as I would like to do this I fear that this won't work.

First of all interesting approach to do this in .get_unmapped_area. The 
standard handling is to have the check like "if 
(is_cow_mapping(vma->vm_flags)) return -EINVAL;", see TTM for example.

Then IIRC we already tried this and had to revert it because it breaks 
the UAPI. Some broken applications actually use shared mappings (but not 
really cow) and we would like to keep them working.

Regards,
Christian.

>
> Signed-off-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> ---
>   drivers/dma-buf/dma-buf.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 21916bba77d5..1ec297241842 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -25,6 +25,7 @@
>   #include <linux/poll.h>
>   #include <linux/dma-resv.h>
>   #include <linux/mm.h>
> +#include <linux/mman.h>
>   #include <linux/mount.h>
>   #include <linux/pseudo_fs.h>
>   
> @@ -128,6 +129,19 @@ static struct file_system_type dma_buf_fs_type = {
>   	.kill_sb = kill_anon_super,
>   };
>   
> +static unsigned long
> +dma_buf_get_unmapped_area(struct file *file,
> +			  unsigned long addr,
> +			  unsigned long len,
> +			  unsigned long pgoff,
> +			  unsigned long flags)
> +{
> +	if ((flags & MAP_TYPE) == MAP_PRIVATE)
> +		return -EINVAL;
> +
> +	return current->mm->get_unmapped_area(file, addr, len, pgoff, flags);
> +}
> +
>   static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
>   {
>   	struct dma_buf *dmabuf;
> @@ -508,6 +522,7 @@ static void dma_buf_show_fdinfo(struct seq_file *m, struct file *file)
>   
>   static const struct file_operations dma_buf_fops = {
>   	.release	= dma_buf_file_release,
> +	.get_unmapped_area = dma_buf_get_unmapped_area,
>   	.mmap		= dma_buf_mmap_internal,
>   	.llseek		= dma_buf_llseek,
>   	.poll		= dma_buf_poll,