* [PATCH 1/2] mm: mmap: fix fput in error path v2 @ 2020-10-12 8:52 Christian König 2020-10-12 8:52 ` [PATCH 2/2] mm: introduce vma_set_file function v4 Christian König ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Christian König @ 2020-10-12 8:52 UTC (permalink / raw) To: akpm, linux-mm, linux-kernel, linaro-mm-sig, dri-devel, linux-media, chris, airlied, daniel, sumit.semwal, willy, jhubbard, jgg, linmiaohe Patch "495c10cc1c0c CHROMIUM: dma-buf: restore args..." adds a workaround for a bug in mmap_region. As the comment states ->mmap() callback can change vma->vm_file and so we might call fput() on the wrong file. Revert the workaround and proper fix this in mmap_region. v2: drop the extra if in dma_buf_mmap as well Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> --- drivers/dma-buf/dma-buf.c | 20 +++----------------- mm/mmap.c | 2 +- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index a6ba4d598f0e..08630d057cf2 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1143,9 +1143,6 @@ EXPORT_SYMBOL_GPL(dma_buf_end_cpu_access); int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, unsigned long pgoff) { - struct file *oldfile; - int ret; - if (WARN_ON(!dmabuf || !vma)) return -EINVAL; @@ -1163,22 +1160,11 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, return -EINVAL; /* readjust the vma */ - get_file(dmabuf->file); - oldfile = vma->vm_file; - vma->vm_file = dmabuf->file; + fput(vma->vm_file); + vma->vm_file = get_file(dmabuf->file); vma->vm_pgoff = pgoff; - ret = dmabuf->ops->mmap(dmabuf, vma); - if (ret) { - /* restore old parameters on failure */ - vma->vm_file = oldfile; - fput(dmabuf->file); - } else { - if (oldfile) - fput(oldfile); - } - return ret; - + return dmabuf->ops->mmap(dmabuf, vma); } EXPORT_SYMBOL_GPL(dma_buf_mmap); diff --git a/mm/mmap.c b/mm/mmap.c index 40248d84ad5f..3a2670d73355 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1852,8 +1852,8 @@ unsigned long mmap_region(struct file *file, unsigned long addr, return addr; unmap_and_free_vma: + fput(vma->vm_file); vma->vm_file = NULL; - fput(file); /* Undo any partial mapping done by a device driver. */ unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end); -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] mm: introduce vma_set_file function v4 2020-10-12 8:52 [PATCH 1/2] mm: mmap: fix fput in error path v2 Christian König @ 2020-10-12 8:52 ` Christian König 2020-10-12 12:15 ` kernel test robot ` (3 more replies) 2020-10-16 16:13 ` [PATCH 1/2] mm: mmap: fix fput in error path v2 Jason Gunthorpe 2020-11-04 8:03 ` Christian König 2 siblings, 4 replies; 8+ messages in thread From: Christian König @ 2020-10-12 8:52 UTC (permalink / raw) To: akpm, linux-mm, linux-kernel, linaro-mm-sig, dri-devel, linux-media, chris, airlied, daniel, sumit.semwal, willy, jhubbard, jgg, linmiaohe Add the new vma_set_file() function to allow changing vma->vm_file with the necessary refcount dance. v2: add more users of this. v3: add missing EXPORT_SYMBOL, rebase on mmap cleanup, add comments why we drop the reference on two occasions. v4: make it clear that changing an anonymous vma is illegal. Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v2) --- drivers/dma-buf/dma-buf.c | 3 +-- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 4 +--- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 3 +-- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 5 +++-- drivers/gpu/drm/msm/msm_gem.c | 4 +--- drivers/gpu/drm/omapdrm/omap_gem.c | 3 +-- drivers/gpu/drm/vgem/vgem_drv.c | 3 +-- drivers/staging/android/ashmem.c | 6 +++--- include/linux/mm.h | 2 ++ mm/mmap.c | 12 ++++++++++++ 10 files changed, 26 insertions(+), 19 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 08630d057cf2..8e6a114c6034 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1160,8 +1160,7 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, return -EINVAL; /* readjust the vma */ - fput(vma->vm_file); - vma->vm_file = get_file(dmabuf->file); + vma_set_file(vma, dmabuf->file); vma->vm_pgoff = pgoff; return dmabuf->ops->mmap(dmabuf, vma); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c index 312e9d58d5a7..10ce267c0947 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c @@ -145,10 +145,8 @@ static int etnaviv_gem_mmap_obj(struct etnaviv_gem_object *etnaviv_obj, * address_space (so unmap_mapping_range does what we want, * in particular in the case of mmap'd dmabufs) */ - fput(vma->vm_file); - get_file(etnaviv_obj->base.filp); vma->vm_pgoff = 0; - vma->vm_file = etnaviv_obj->base.filp; + vma_set_file(vma, etnaviv_obj->base.filp); vma->vm_page_prot = vm_page_prot; } diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index fec0e1e3dc3e..8ce4c9e28b87 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -119,8 +119,7 @@ static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct * if (ret) return ret; - fput(vma->vm_file); - vma->vm_file = get_file(obj->base.filp); + vma_set_file(vma, obj->base.filp); return 0; } diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c index 3d69e51f3e4d..ec28a6cde49b 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c @@ -893,8 +893,9 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma) * requires avoiding extraneous references to their filp, hence why * we prefer to use an anonymous file for their mmaps. */ - fput(vma->vm_file); - vma->vm_file = anon; + vma_set_file(vma, anon); + /* Drop the initial creation reference, the vma is now holding one. */ + fput(anon); switch (mmo->mmap_type) { case I915_MMAP_TYPE_WC: diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index de915ff6f4b4..a71f42870d5e 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -223,10 +223,8 @@ int msm_gem_mmap_obj(struct drm_gem_object *obj, * address_space (so unmap_mapping_range does what we want, * in particular in the case of mmap'd dmabufs) */ - fput(vma->vm_file); - get_file(obj->filp); vma->vm_pgoff = 0; - vma->vm_file = obj->filp; + vma_set_file(vma, obj->filp); vma->vm_page_prot = vm_get_page_prot(vma->vm_flags); } diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c index 979d53a93c2b..0d4542ff1d7d 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem.c +++ b/drivers/gpu/drm/omapdrm/omap_gem.c @@ -564,9 +564,8 @@ int omap_gem_mmap_obj(struct drm_gem_object *obj, * address_space (so unmap_mapping_range does what we want, * in particular in the case of mmap'd dmabufs) */ - fput(vma->vm_file); vma->vm_pgoff = 0; - vma->vm_file = get_file(obj->filp); + vma_set_file(vma, obj->filp); vma->vm_page_prot = vm_get_page_prot(vma->vm_flags); } diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c index fa54a6d1403d..ea0eecae5153 100644 --- a/drivers/gpu/drm/vgem/vgem_drv.c +++ b/drivers/gpu/drm/vgem/vgem_drv.c @@ -397,8 +397,7 @@ static int vgem_prime_mmap(struct drm_gem_object *obj, if (ret) return ret; - fput(vma->vm_file); - vma->vm_file = get_file(obj->filp); + vma_set_file(vma, obj->filp); vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP; vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c index 10b4be1f3e78..4789d36ddfd3 100644 --- a/drivers/staging/android/ashmem.c +++ b/drivers/staging/android/ashmem.c @@ -450,9 +450,9 @@ static int ashmem_mmap(struct file *file, struct vm_area_struct *vma) vma_set_anonymous(vma); } - if (vma->vm_file) - fput(vma->vm_file); - vma->vm_file = asma->file; + vma_set_file(vma, asma->file); + /* XXX: merge this with the get_file() above if possible */ + fput(asma->file); out: mutex_unlock(&ashmem_mutex); diff --git a/include/linux/mm.h b/include/linux/mm.h index ca6e6a81576b..f7a005153d02 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -2693,6 +2693,8 @@ static inline void vma_set_page_prot(struct vm_area_struct *vma) } #endif +void vma_set_file(struct vm_area_struct *vma, struct file *file); + #ifdef CONFIG_NUMA_BALANCING unsigned long change_prot_numa(struct vm_area_struct *vma, unsigned long start, unsigned long end); diff --git a/mm/mmap.c b/mm/mmap.c index 3a2670d73355..8634d0bb54ad 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -136,6 +136,18 @@ void vma_set_page_prot(struct vm_area_struct *vma) WRITE_ONCE(vma->vm_page_prot, vm_page_prot); } +/* + * Change backing file, only valid to use during initial VMA setup. + */ +void vma_set_file(struct vm_area_struct *vma, struct file *file) +{ + /* Changing an anonymous vma with this is illegal */ + get_file(file); + swap(vma->vm_file, file); + fput(file); +} +EXPORT_SYMBOL(vma_set_file); + /* * Requires inode->i_mapping->i_mmap_rwsem */ -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mm: introduce vma_set_file function v4 2020-10-12 8:52 ` [PATCH 2/2] mm: introduce vma_set_file function v4 Christian König @ 2020-10-12 12:15 ` kernel test robot 2020-10-12 14:08 ` kernel test robot ` (2 subsequent siblings) 3 siblings, 0 replies; 8+ messages in thread From: kernel test robot @ 2020-10-12 12:15 UTC (permalink / raw) To: Christian König, akpm, linux-mm, linux-kernel, linaro-mm-sig, dri-devel, linux-media, chris, airlied, daniel, sumit.semwal Cc: kbuild-all, clang-built-linux [-- Attachment #1: Type: text/plain, Size: 1924 bytes --] Hi "Christian, I love your patch! Yet something to improve: [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on staging/staging-testing linus/master v5.9 next-20201009] [cannot apply to mmotm/master] [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] url: https://github.com/0day-ci/linux/commits/Christian-K-nig/mm-mmap-fix-fput-in-error-path-v2/20201012-165336 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: arm-randconfig-r025-20201012 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 9e72d3eaf38f217698f72cb8fdc969a6e72dad3a) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm cross compiling tool for clang build # apt-get install binutils-arm-linux-gnueabi # https://github.com/0day-ci/linux/commit/4ff869f185acba6d9c37ab6abdb0d9f93f31d15b git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Christian-K-nig/mm-mmap-fix-fput-in-error-path-v2/20201012-165336 git checkout 4ff869f185acba6d9c37ab6abdb0d9f93f31d15b # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> ld.lld: error: undefined symbol: vma_set_file >>> referenced by dma-buf.c >>> dma-buf/dma-buf.o:(dma_buf_mmap) in archive drivers/built-in.a --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 32310 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mm: introduce vma_set_file function v4 2020-10-12 8:52 ` [PATCH 2/2] mm: introduce vma_set_file function v4 Christian König 2020-10-12 12:15 ` kernel test robot @ 2020-10-12 14:08 ` kernel test robot 2020-10-12 14:22 ` kernel test robot 2020-10-16 16:13 ` Jason Gunthorpe 3 siblings, 0 replies; 8+ messages in thread From: kernel test robot @ 2020-10-12 14:08 UTC (permalink / raw) To: Christian König, akpm, linux-mm, linux-kernel, linaro-mm-sig, dri-devel, linux-media, chris, airlied, daniel, sumit.semwal Cc: kbuild-all [-- Attachment #1: Type: text/plain, Size: 1726 bytes --] Hi "Christian, I love your patch! Yet something to improve: [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on staging/staging-testing linus/master hnaz-linux-mm/master v5.9 next-20201012] [cannot apply to mmotm/master] [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] url: https://github.com/0day-ci/linux/commits/Christian-K-nig/mm-mmap-fix-fput-in-error-path-v2/20201012-165336 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: sh-allmodconfig (attached as .config) compiler: sh4-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/4ff869f185acba6d9c37ab6abdb0d9f93f31d15b git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Christian-K-nig/mm-mmap-fix-fput-in-error-path-v2/20201012-165336 git checkout 4ff869f185acba6d9c37ab6abdb0d9f93f31d15b # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sh If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): sh4-linux-ld: drivers/dma-buf/dma-buf.o: in function `dma_buf_mmap': >> (.text+0x8c4): undefined reference to `vma_set_file' --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 52717 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mm: introduce vma_set_file function v4 2020-10-12 8:52 ` [PATCH 2/2] mm: introduce vma_set_file function v4 Christian König 2020-10-12 12:15 ` kernel test robot 2020-10-12 14:08 ` kernel test robot @ 2020-10-12 14:22 ` kernel test robot 2020-10-16 16:13 ` Jason Gunthorpe 3 siblings, 0 replies; 8+ messages in thread From: kernel test robot @ 2020-10-12 14:22 UTC (permalink / raw) To: Christian König, akpm, linux-mm, linux-kernel, linaro-mm-sig, dri-devel, linux-media, chris, airlied, daniel, sumit.semwal Cc: kbuild-all [-- Attachment #1: Type: text/plain, Size: 2554 bytes --] Hi "Christian, I love your patch! Yet something to improve: [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on staging/staging-testing linus/master v5.9 next-20201012] [cannot apply to mmotm/master] [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] url: https://github.com/0day-ci/linux/commits/Christian-K-nig/mm-mmap-fix-fput-in-error-path-v2/20201012-165336 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: arm-randconfig-r034-20201012 (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/4ff869f185acba6d9c37ab6abdb0d9f93f31d15b git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Christian-K-nig/mm-mmap-fix-fput-in-error-path-v2/20201012-165336 git checkout 4ff869f185acba6d9c37ab6abdb0d9f93f31d15b # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): arm-linux-gnueabi-ld: drivers/gpu/drm/vgem/vgem_drv.o: in function `vgem_prime_mmap': >> drivers/gpu/drm/vgem/vgem_drv.c:396: undefined reference to `vma_set_file' arm-linux-gnueabi-ld: drivers/dma-buf/dma-buf.o: in function `dma_buf_mmap': >> drivers/dma-buf/dma-buf.c:1163: undefined reference to `vma_set_file' vim +396 drivers/gpu/drm/vgem/vgem_drv.c 380 381 static int vgem_prime_mmap(struct drm_gem_object *obj, 382 struct vm_area_struct *vma) 383 { 384 int ret; 385 386 if (obj->size < vma->vm_end - vma->vm_start) 387 return -EINVAL; 388 389 if (!obj->filp) 390 return -ENODEV; 391 392 ret = call_mmap(obj->filp, vma); 393 if (ret) 394 return ret; 395 > 396 vma_set_file(vma, obj->filp); 397 vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP; 398 vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags)); 399 400 return 0; 401 } 402 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 28955 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mm: introduce vma_set_file function v4 2020-10-12 8:52 ` [PATCH 2/2] mm: introduce vma_set_file function v4 Christian König ` (2 preceding siblings ...) 2020-10-12 14:22 ` kernel test robot @ 2020-10-16 16:13 ` Jason Gunthorpe 3 siblings, 0 replies; 8+ messages in thread From: Jason Gunthorpe @ 2020-10-16 16:13 UTC (permalink / raw) To: Christian König Cc: akpm, linux-mm, linux-kernel, linaro-mm-sig, dri-devel, linux-media, chris, airlied, daniel, sumit.semwal, willy, jhubbard, linmiaohe On Mon, Oct 12, 2020 at 10:52:03AM +0200, Christian König wrote: > Add the new vma_set_file() function to allow changing > vma->vm_file with the necessary refcount dance. > > v2: add more users of this. > v3: add missing EXPORT_SYMBOL, rebase on mmap cleanup, > add comments why we drop the reference on two occasions. > v4: make it clear that changing an anonymous vma is illegal. > > Signed-off-by: Christian König <christian.koenig@amd.com> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (v2) > --- > drivers/dma-buf/dma-buf.c | 3 +-- > drivers/gpu/drm/etnaviv/etnaviv_gem.c | 4 +--- > drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 3 +-- > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 5 +++-- > drivers/gpu/drm/msm/msm_gem.c | 4 +--- > drivers/gpu/drm/omapdrm/omap_gem.c | 3 +-- > drivers/gpu/drm/vgem/vgem_drv.c | 3 +-- > drivers/staging/android/ashmem.c | 6 +++--- > include/linux/mm.h | 2 ++ > mm/mmap.c | 12 ++++++++++++ > 10 files changed, 26 insertions(+), 19 deletions(-) Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Jason ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] mm: mmap: fix fput in error path v2 2020-10-12 8:52 [PATCH 1/2] mm: mmap: fix fput in error path v2 Christian König 2020-10-12 8:52 ` [PATCH 2/2] mm: introduce vma_set_file function v4 Christian König @ 2020-10-16 16:13 ` Jason Gunthorpe 2020-11-04 8:03 ` Christian König 2 siblings, 0 replies; 8+ messages in thread From: Jason Gunthorpe @ 2020-10-16 16:13 UTC (permalink / raw) To: Christian König Cc: akpm, linux-mm, linux-kernel, linaro-mm-sig, dri-devel, linux-media, chris, airlied, daniel, sumit.semwal, willy, jhubbard, linmiaohe On Mon, Oct 12, 2020 at 10:52:02AM +0200, Christian König wrote: > Patch "495c10cc1c0c CHROMIUM: dma-buf: restore args..." > adds a workaround for a bug in mmap_region. > > As the comment states ->mmap() callback can change > vma->vm_file and so we might call fput() on the wrong file. > > Revert the workaround and proper fix this in mmap_region. > > v2: drop the extra if in dma_buf_mmap as well > > Signed-off-by: Christian König <christian.koenig@amd.com> > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> > --- > drivers/dma-buf/dma-buf.c | 20 +++----------------- > mm/mmap.c | 2 +- > 2 files changed, 4 insertions(+), 18 deletions(-) Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Probably should Fixes that other patch Andrew pointed at Jason ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] mm: mmap: fix fput in error path v2 2020-10-12 8:52 [PATCH 1/2] mm: mmap: fix fput in error path v2 Christian König 2020-10-12 8:52 ` [PATCH 2/2] mm: introduce vma_set_file function v4 Christian König 2020-10-16 16:13 ` [PATCH 1/2] mm: mmap: fix fput in error path v2 Jason Gunthorpe @ 2020-11-04 8:03 ` Christian König 2 siblings, 0 replies; 8+ messages in thread From: Christian König @ 2020-11-04 8:03 UTC (permalink / raw) To: akpm, linux-mm, linux-kernel, linaro-mm-sig, dri-devel, linux-media, chris, airlied, daniel, sumit.semwal, willy, jhubbard, jgg, linmiaohe If nobody comes up with an objections I'm going to merge that through drm-misc-next. Thanks, Christian. Am 12.10.20 um 10:52 schrieb Christian König: > Patch "495c10cc1c0c CHROMIUM: dma-buf: restore args..." > adds a workaround for a bug in mmap_region. > > As the comment states ->mmap() callback can change > vma->vm_file and so we might call fput() on the wrong file. > > Revert the workaround and proper fix this in mmap_region. > > v2: drop the extra if in dma_buf_mmap as well > > Signed-off-by: Christian König <christian.koenig@amd.com> > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> > --- > drivers/dma-buf/dma-buf.c | 20 +++----------------- > mm/mmap.c | 2 +- > 2 files changed, 4 insertions(+), 18 deletions(-) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index a6ba4d598f0e..08630d057cf2 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -1143,9 +1143,6 @@ EXPORT_SYMBOL_GPL(dma_buf_end_cpu_access); > int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, > unsigned long pgoff) > { > - struct file *oldfile; > - int ret; > - > if (WARN_ON(!dmabuf || !vma)) > return -EINVAL; > > @@ -1163,22 +1160,11 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, > return -EINVAL; > > /* readjust the vma */ > - get_file(dmabuf->file); > - oldfile = vma->vm_file; > - vma->vm_file = dmabuf->file; > + fput(vma->vm_file); > + vma->vm_file = get_file(dmabuf->file); > vma->vm_pgoff = pgoff; > > - ret = dmabuf->ops->mmap(dmabuf, vma); > - if (ret) { > - /* restore old parameters on failure */ > - vma->vm_file = oldfile; > - fput(dmabuf->file); > - } else { > - if (oldfile) > - fput(oldfile); > - } > - return ret; > - > + return dmabuf->ops->mmap(dmabuf, vma); > } > EXPORT_SYMBOL_GPL(dma_buf_mmap); > > diff --git a/mm/mmap.c b/mm/mmap.c > index 40248d84ad5f..3a2670d73355 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -1852,8 +1852,8 @@ unsigned long mmap_region(struct file *file, unsigned long addr, > return addr; > > unmap_and_free_vma: > + fput(vma->vm_file); > vma->vm_file = NULL; > - fput(file); > > /* Undo any partial mapping done by a device driver. */ > unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end); ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-11-04 8:03 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-12 8:52 [PATCH 1/2] mm: mmap: fix fput in error path v2 Christian König 2020-10-12 8:52 ` [PATCH 2/2] mm: introduce vma_set_file function v4 Christian König 2020-10-12 12:15 ` kernel test robot 2020-10-12 14:08 ` kernel test robot 2020-10-12 14:22 ` kernel test robot 2020-10-16 16:13 ` Jason Gunthorpe 2020-10-16 16:13 ` [PATCH 1/2] mm: mmap: fix fput in error path v2 Jason Gunthorpe 2020-11-04 8:03 ` Christian König
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).