- Linaro-mm-sig - lists.linaro.org

[bug report] misc: fastrpc: Add support for context Invoke method

by Dan Carpenter

Hello Srinivas Kandagatla, The patch c68cfb718c8f: "misc: fastrpc: Add support for context Invoke method" from Feb 8, 2019, leads to the following static checker warning: drivers/misc/fastrpc.c:990 fastrpc_internal_invoke() warn: 'ctx->refcount.refcount.ref.counter' not decremented on lines: 990. drivers/misc/fastrpc.c 925 static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel, 926 u32 handle, u32 sc, 927 struct fastrpc_invoke_args *args) 928 { 929 struct fastrpc_invoke_ctx *ctx = NULL; 930 int err = 0; 931 932 if (!fl->sctx) 933 return -EINVAL; 934 935 if (!fl->cctx->rpdev) 936 return -EPIPE; 937 938 ctx = fastrpc_context_alloc(fl, kernel, sc, args); refcount is 1. 939 if (IS_ERR(ctx)) 940 return PTR_ERR(ctx); 941 942 if (ctx->nscalars) { 943 err = fastrpc_get_args(kernel, ctx); 944 if (err) 945 goto bail; ^^^^^^^^^ Still holding one refcount. 946 } 947 948 /* make sure that all CPU memory writes are seen by DSP */ 949 dma_wmb(); 950 /* Send invoke buffer to remote dsp */ 951 err = fastrpc_invoke_send(fl->sctx, ctx, kernel, handle); ^^^^^^^^^^^^^^^^^^^ Takes a reference count. Refcount is now 2. 952 if (err) 953 goto bail; 954 955 if (kernel) { 956 if (!wait_for_completion_timeout(&ctx->work, 10 * HZ)) 957 err = -ETIMEDOUT; 958 } else { 959 err = wait_for_completion_interruptible(&ctx->work); This drops a refcount. 960 } 961 962 if (err) 963 goto bail; 964 965 /* Check the response from remote dsp */ 966 err = ctx->retval; 967 if (err) 968 goto bail; 969 970 if (ctx->nscalars) { 971 /* make sure that all memory writes by DSP are seen by CPU */ 972 dma_rmb(); 973 /* populate all the output buffers with results */ 974 err = fastrpc_put_args(ctx, kernel); 975 if (err) 976 goto bail; 977 } 978 979 bail: 980 if (err != -ERESTARTSYS && err != -ETIMEDOUT) { 981 /* We are done with this compute context */ 982 spin_lock(&fl->lock); 983 list_del(&ctx->node); 984 spin_unlock(&fl->lock); 985 fastrpc_context_put(ctx); If we're holding two refcounts then I think this leaks. 986 } 987 if (err) 988 dev_dbg(fl->sctx->dev, "Error: Invoke Failed %d\n", err); 989 990 return err; 991 } regards, dan carpenter

5 years

1
0
0 0

[PATCH 2/3] dma-fence: use default wait function for mock fences

by Daniel Vetter

No need to micro-optmize when we're waiting in a mocked object ... Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org --- drivers/dma-buf/st-dma-fence.c | 41 ---------------------------------- 1 file changed, 41 deletions(-) diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c index e593064341c8..8166d2984702 100644 --- a/drivers/dma-buf/st-dma-fence.c +++ b/drivers/dma-buf/st-dma-fence.c @@ -33,50 +33,9 @@ static void mock_fence_release(struct dma_fence *f) kmem_cache_free(slab_fences, to_mock_fence(f)); } -struct wait_cb { - struct dma_fence_cb cb; - struct task_struct *task; -}; - -static void mock_wakeup(struct dma_fence *f, struct dma_fence_cb *cb) -{ - wake_up_process(container_of(cb, struct wait_cb, cb)->task); -} - -static long mock_wait(struct dma_fence *f, bool intr, long timeout) -{ - const int state = intr ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE; - struct wait_cb cb = { .task = current }; - - if (dma_fence_add_callback(f, &cb.cb, mock_wakeup)) - return timeout; - - while (timeout) { - set_current_state(state); - - if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &f->flags)) - break; - - if (signal_pending_state(state, current)) - break; - - timeout = schedule_timeout(timeout); - } - __set_current_state(TASK_RUNNING); - - if (!dma_fence_remove_callback(f, &cb.cb)) - return timeout; - - if (signal_pending_state(state, current)) - return -ERESTARTSYS; - - return -ETIME; -} - static const struct dma_fence_ops mock_ops = { .get_driver_name = mock_name, .get_timeline_name = mock_name, - .wait = mock_wait, .release = mock_fence_release, }; -- 2.26.2

5 years

3
2
0 0

[PATCH 3/3] misc/habalabs: don't set default fence_ops->wait

by Daniel Vetter

It's the default. Also so much for "we're not going to tell the graphics people how to review their code", dma_fence is a pretty core piece of gpu driver infrastructure. And it's very much uapi relevant, including piles of corresponding userspace protocols and libraries for how to pass these around. Would be great if habanalabs would not use this (from a quick look it's not needed at all), since open source the userspace and playing by the usual rules isn't on the table. If that's not possible (because it's actually using the uapi part of dma_fence to interact with gpu drivers) then we have exactly what everyone promised we'd want to avoid. Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> Cc: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Cc: Olof Johansson <olof(a)lixom.net> Cc: Oded Gabbay <oded.gabbay(a)gmail.com> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org --- drivers/misc/habanalabs/command_submission.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/misc/habanalabs/command_submission.c b/drivers/misc/habanalabs/command_submission.c index 409276b6374d..cc3ce759b6c3 100644 --- a/drivers/misc/habanalabs/command_submission.c +++ b/drivers/misc/habanalabs/command_submission.c @@ -46,7 +46,6 @@ static const struct dma_fence_ops hl_fence_ops = { .get_driver_name = hl_fence_get_driver_name, .get_timeline_name = hl_fence_get_timeline_name, .enable_signaling = hl_fence_enable_signaling, - .wait = dma_fence_default_wait, .release = hl_fence_release }; -- 2.26.2

5 years

1
0
0 0

Re: [Linaro-mm-sig] [PATCH] dma-buf: heaps: Remove Unneeded variable "ret" in dma_heap_dma_buf_begin_cpu_access()

by Brian Starkey

Hi Jason, On Wed, May 06, 2020 at 02:18:51PM +0800, Jason Yan wrote: > Fix the following coccicheck warning: > > drivers/dma-buf/heaps/heap-helpers.c:203:5-8: Unneeded variable: "ret". > Return "0" on line 216 > > Signed-off-by: Jason Yan <yanaijie(a)huawei.com> LGTM. Reviewed-by: Brian Starkey <brian.starkey(a)arm.com> Thanks, -Brian > --- > drivers/dma-buf/heaps/heap-helpers.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/dma-buf/heaps/heap-helpers.c b/drivers/dma-buf/heaps/heap-helpers.c > index 9f964ca3f59c..c82872501ba2 100644 > --- a/drivers/dma-buf/heaps/heap-helpers.c > +++ b/drivers/dma-buf/heaps/heap-helpers.c > @@ -200,7 +200,6 @@ static int dma_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, > { > struct heap_helper_buffer *buffer = dmabuf->priv; > struct dma_heaps_attachment *a; > - int ret = 0; > > mutex_lock(&buffer->lock); > > @@ -213,7 +212,7 @@ static int dma_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, > } > mutex_unlock(&buffer->lock); > > - return ret; > + return 0; > } > > static int dma_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf, > -- > 2.21.1 >

5 years, 1 month

1
0
0 0

Re: [Linaro-mm-sig] [PATCH] dma-buf: fix use-after-free in dmabuffs_dname

by Greg KH

On Wed, May 06, 2020 at 02:00:10PM +0530, Charan Teja Kalla wrote: > Thank you Greg for the reply. > > On 5/5/2020 3:38 PM, Greg KH wrote: > > On Tue, Apr 28, 2020 at 01:24:02PM +0530, Charan Teja Reddy wrote: > > > The following race occurs while accessing the dmabuf object exported as > > > file: > > > P1 P2 > > > dma_buf_release() dmabuffs_dname() > > > [say lsof reading /proc/<P1 pid>/fd/<num>] > > > > > > read dmabuf stored in dentry->fsdata > > > Free the dmabuf object > > > Start accessing the dmabuf structure > > > > > > In the above description, the dmabuf object freed in P1 is being > > > accessed from P2 which is resulting into the use-after-free. Below is > > > the dump stack reported. > > > > > > Call Trace: > > > kasan_report+0x12/0x20 > > > __asan_report_load8_noabort+0x14/0x20 > > > dmabuffs_dname+0x4f4/0x560 > > > tomoyo_realpath_from_path+0x165/0x660 > > > tomoyo_get_realpath > > > tomoyo_check_open_permission+0x2a3/0x3e0 > > > tomoyo_file_open > > > tomoyo_file_open+0xa9/0xd0 > > > security_file_open+0x71/0x300 > > > do_dentry_open+0x37a/0x1380 > > > vfs_open+0xa0/0xd0 > > > path_openat+0x12ee/0x3490 > > > do_filp_open+0x192/0x260 > > > do_sys_openat2+0x5eb/0x7e0 > > > do_sys_open+0xf2/0x180 > > > > > > Fixes: bb2bb90 ("dma-buf: add DMA_BUF_SET_NAME ioctls") > > Nit, please read the documentation for how to do a Fixes: line properly, > > you need more digits: > > Fixes: bb2bb9030425 ("dma-buf: add DMA_BUF_SET_NAME ioctls") > > > Will update the patch > > > > > Reported-by:syzbot+3643a18836bce555bff6@syzkaller.appspotmail.com > > > Signed-off-by: Charan Teja Reddy<charante(a)codeaurora.org> > > Also, any reason you didn't include the other mailing lists that > > get_maintainer.pl said to? > > > Really sorry for not sending to complete list. Added now. > > > > And finally, no cc: stable in the s-o-b area for an issue that needs to > > be backported to older kernels? > > > Will update the patch. > > > > > > > --- > > > drivers/dma-buf/dma-buf.c | 25 +++++++++++++++++++++++-- > > > include/linux/dma-buf.h | 1 + > > > 2 files changed, 24 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > > > index 570c923..069d8f78 100644 > > > --- a/drivers/dma-buf/dma-buf.c > > > +++ b/drivers/dma-buf/dma-buf.c > > > @@ -25,6 +25,7 @@ > > > #include <linux/mm.h> > > > #include <linux/mount.h> > > > #include <linux/pseudo_fs.h> > > > +#include <linux/dcache.h> > > > #include <uapi/linux/dma-buf.h> > > > #include <uapi/linux/magic.h> > > > @@ -38,18 +39,34 @@ struct dma_buf_list { > > > static struct dma_buf_list db_list; > > > +static void dmabuf_dent_put(struct dma_buf *dmabuf) > > > +{ > > > + if (atomic_dec_and_test(&dmabuf->dent_count)) { > > > + kfree(dmabuf->name); > > > + kfree(dmabuf); > > > + } > > Why not just use a kref instead of an open-coded atomic value? > > > Kref approach looks cleaner. will update the patch accordingly. > > > > > +} > > > + > > > static char *dmabuffs_dname(struct dentry *dentry, char *buffer, int buflen) > > > { > > > struct dma_buf *dmabuf; > > > char name[DMA_BUF_NAME_LEN]; > > > size_t ret = 0; > > > + spin_lock(&dentry->d_lock); > > > dmabuf = dentry->d_fsdata; > > > + if (!dmabuf || !atomic_add_unless(&dmabuf->dent_count, 1, 0)) { > > > + spin_unlock(&dentry->d_lock); > > > + goto out; > > How can dmabuf not be valid here? > > > > And isn't there already a usecount for the buffer? > > > dmabuf exported as file simply relies on that file's refcount, thus fput() > releases the dmabuf. > > We are storing the dmabuf in the dentry->d_fsdata but there is no binding > between the dentry and the dmabuf. So, flow will be like > > 1) P1 calls fput(dmabuf_fd) > > 2) P2 trying to access the file information of P1. > Eg: lsof command trying to list out the dmabuf_fd information using > /proc/<P1 pid>/fd/dmabuf_fd > > 3) P1 calls the file->f_op->release(dmabuf_fd_file)(ends up in calling > dma_buf_release()), thus frees up the dmabuf buffer. > > 4) P2 access the dmabuf stored in the dentry->d_fsdata which was freed in > step 3. > > So we need to have some refcount mechanism to avoid the use-after-free in > step 4. Ok, but watch out, now you have 2 different reference counts for the same structure. Keeping them coordinated is almost always an impossible task so you need to only rely on one. If you can't use the file api, just drop all of the reference counting logic in there and only use the kref one. good luck! greg k-h

5 years, 1 month

1
0
0 0

[PATCH 5.6 02/73] dma-buf: Fix SET_NAME ioctl uapi

by Greg Kroah-Hartman

From: Daniel Vetter <daniel.vetter(a)intel.com> commit a5bff92eaac45bdf6221badf9505c26792fdf99e upstream. The uapi is the same on 32 and 64 bit, but the number isn't. Everyone who botched this please re-read: https://www.kernel.org/doc/html/v5.4-preprc-cpu/ioctl/botching-up-ioctls.ht… Also, the type argument for the ioctl macros is for the type the void __user *arg pointer points at, which in this case would be the variable-sized char[] of a 0 terminated string. So this was botched in more than just the usual ways. Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: Chenbo Feng <fengc(a)google.com> Cc: Greg Hackmann <ghackmann(a)google.com> Cc: Daniel Vetter <daniel.vetter(a)ffwll.ch> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: minchan(a)kernel.org Cc: surenb(a)google.com Cc: jenhaochen(a)google.com Cc: Martin Liu <liumartin(a)google.com> Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> Tested-by: Martin Liu <liumartin(a)google.com> Reviewed-by: Martin Liu <liumartin(a)google.com> Signed-off-by: Sumit Semwal <sumit.semwal(a)linaro.org> [sumits: updated some checkpatch fixes, corrected author email] Link: https://patchwork.freedesktop.org/patch/msgid/20200407133002.3486387-1-dani… Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/dma-buf/dma-buf.c | 3 ++- include/uapi/linux/dma-buf.h | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -388,7 +388,8 @@ static long dma_buf_ioctl(struct file *f return ret; - case DMA_BUF_SET_NAME: + case DMA_BUF_SET_NAME_A: + case DMA_BUF_SET_NAME_B: return dma_buf_set_name(dmabuf, (const char __user *)arg); default: --- a/include/uapi/linux/dma-buf.h +++ b/include/uapi/linux/dma-buf.h @@ -39,6 +39,12 @@ struct dma_buf_sync { #define DMA_BUF_BASE 'b' #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync) + +/* 32/64bitness of this uapi was botched in android, there's no difference + * between them in actual uapi, they're just different numbers. + */ #define DMA_BUF_SET_NAME _IOW(DMA_BUF_BASE, 1, const char *) +#define DMA_BUF_SET_NAME_A _IOW(DMA_BUF_BASE, 1, u32) +#define DMA_BUF_SET_NAME_B _IOW(DMA_BUF_BASE, 1, u64) #endif

5 years, 1 month

1
0
0 0

[PATCH 5.4 01/57] dma-buf: Fix SET_NAME ioctl uapi

by Greg Kroah-Hartman

From: Daniel Vetter <daniel.vetter(a)intel.com> commit a5bff92eaac45bdf6221badf9505c26792fdf99e upstream. The uapi is the same on 32 and 64 bit, but the number isn't. Everyone who botched this please re-read: https://www.kernel.org/doc/html/v5.4-preprc-cpu/ioctl/botching-up-ioctls.ht… Also, the type argument for the ioctl macros is for the type the void __user *arg pointer points at, which in this case would be the variable-sized char[] of a 0 terminated string. So this was botched in more than just the usual ways. Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: Chenbo Feng <fengc(a)google.com> Cc: Greg Hackmann <ghackmann(a)google.com> Cc: Daniel Vetter <daniel.vetter(a)ffwll.ch> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: minchan(a)kernel.org Cc: surenb(a)google.com Cc: jenhaochen(a)google.com Cc: Martin Liu <liumartin(a)google.com> Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> Tested-by: Martin Liu <liumartin(a)google.com> Reviewed-by: Martin Liu <liumartin(a)google.com> Signed-off-by: Sumit Semwal <sumit.semwal(a)linaro.org> [sumits: updated some checkpatch fixes, corrected author email] Link: https://patchwork.freedesktop.org/patch/msgid/20200407133002.3486387-1-dani… Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/dma-buf/dma-buf.c | 3 ++- include/uapi/linux/dma-buf.h | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -388,7 +388,8 @@ static long dma_buf_ioctl(struct file *f return ret; - case DMA_BUF_SET_NAME: + case DMA_BUF_SET_NAME_A: + case DMA_BUF_SET_NAME_B: return dma_buf_set_name(dmabuf, (const char __user *)arg); default: --- a/include/uapi/linux/dma-buf.h +++ b/include/uapi/linux/dma-buf.h @@ -39,6 +39,12 @@ struct dma_buf_sync { #define DMA_BUF_BASE 'b' #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync) + +/* 32/64bitness of this uapi was botched in android, there's no difference + * between them in actual uapi, they're just different numbers. + */ #define DMA_BUF_SET_NAME _IOW(DMA_BUF_BASE, 1, const char *) +#define DMA_BUF_SET_NAME_A _IOW(DMA_BUF_BASE, 1, u32) +#define DMA_BUF_SET_NAME_B _IOW(DMA_BUF_BASE, 1, u64) #endif

5 years, 1 month

1
0
0 0

[PATCH v2 00/21] DRM: fix struct sg_table nents vs. orig_nents misuse

by Marek Szyprowski

Dear All, During the Exynos DRM GEM rework and fixing the issues in the drm_prime_sg_to_page_addr_arrays() function [1] I've noticed that most drivers in DRM framework incorrectly use nents and orig_nents entries of the struct sg_table. In case of the most DMA-mapping implementations exchanging those two entries or using nents for all loops on the scatterlist is harmless, because they both have the same value. There exists however a DMA-mapping implementations, for which such incorrect usage breaks things. The nents returned by dma_map_sg() might be lower than the nents passed as its parameter and this is perfectly fine. DMA framework or IOMMU is allowed to join consecutive chunks while mapping if such operation is supported by the underlying HW (bus, bridge, IOMMU, etc). Example of the case where dma_map_sg() might return 1 'DMA' chunk for the 4 'physical' pages is described here [2] The DMA-mapping framework documentation [3] states that dma_map_sg() returns the numer of the created entries in the DMA address space. However the subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be called with the original number of entries passed to dma_map_sg. The common pattern in DRM drivers were to assign the dma_map_sg() return value to sg_table->nents and use that value for the subsequent calls to dma_sync_sg_* or dma_unmap_sg functions. Also the code iterated over nents times to access the pages stored in the processed scatterlist, while it should use orig_nents as the numer of the page entries. I've tried to identify all such incorrect usage of sg_table->nents and this is a result of my research. It looks that the incorrect pattern has been copied over the many drivers mainly in the DRM subsystem. Too bad in most cases it even worked correctly if the system used simple, linear DMA-mapping implementation, for which swapping nents and orig_nents doesn't make any difference. The biggest TODO is DRM/i915 driver and I don't feel brave enough to fix it fully. The driver creatively uses sg_table->orig_nents to store the size of the allocate scatterlist and ignores the number of the entries returned by dma_map_sg function. In this patchset I only fixed the sg_table objects exported by dmabuf related functions. I hope that I didn't break anything there. As a follow-up for this patchset I will prepare a common dma_{map,sync,unmap}_sgtable() wrappers, which will use a proper sg_table entries and call respective DMA-mapping functions. I hope this will help to avoid issue like this in the future. Patches are based on top of Linux next-20200504. Best regards, Marek Szyprowski References: [1] https://lkml.org/lkml/2020/3/27/555 [2] https://lkml.org/lkml/2020/3/29/65 [3] Documentation/DMA-API-HOWTO.txt Changelog: v2: - dropped most of the changes to drm/i915 - added fixes for rcar-du, xen, media and ion - fixed a few issues pointed by kbuild test robot - added wide cc: list for each patch v1: https://lore.kernel.org/linux-iommu/c01c9766-9778-fd1f-f36e-2dc7bd376ba4@ar… - initial version Patch summary: Marek Szyprowski (21): drm: core: fix sg_table nents vs. orig_nents misuse drm: amdgpu: fix sg_table nents vs. orig_nents misuse drm: armada: fix sg_table nents vs. orig_nents misuse drm: etnaviv: fix sg_table nents vs. orig_nents misuse drm: exynos: fix sg_table nents vs. orig_nents misuse drm: i915: fix sg_table nents vs. orig_nents misuse for dmabuf objects drm: lima: fix sg_table nents vs. orig_nents misuse drm: msm: fix sg_table nents vs. orig_nents misuse drm: panfrost: fix sg_table nents vs. orig_nents misuse drm: radeon: fix sg_table nents vs. orig_nents misuse drm: rockchip: fix sg_table nents vs. orig_nents misuse drm: tegra: fix sg_table nents vs. orig_nents misuse drm: virtio: fix sg_table nents vs. orig_nents misuse drm: vmwgfx: fix sg_table nents vs. orig_nents misuse drm: xen: fix sg_table nents vs. orig_nents misuse drm: host1x: fix sg_table nents vs. orig_nents misuse drm: rcar-du: fix sg_table nents vs. orig_nents misuse xen: gntdev: fix sg_table nents vs. orig_nents misuse dmabuf: fix sg_table nents vs. orig_nents misuse media: pci: fix common ALSA DMA-mapping related code staging: ion: fix sg_table nents vs. orig_nents misuse drivers/dma-buf/heaps/heap-helpers.c | 7 ++++--- drivers/dma-buf/udmabuf.c | 5 +++-- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 7 ++++--- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 ++++---- drivers/gpu/drm/armada/armada_gem.c | 14 ++++++++----- drivers/gpu/drm/drm_cache.c | 2 +- drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++--- drivers/gpu/drm/drm_prime.c | 9 +++++---- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 10 ++++++---- drivers/gpu/drm/exynos/exynos_drm_g2d.c | 7 ++++--- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 9 +++++---- drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c | 5 +++-- drivers/gpu/drm/lima/lima_gem.c | 4 ++-- drivers/gpu/drm/msm/msm_gem.c | 8 ++++---- drivers/gpu/drm/msm/msm_iommu.c | 3 ++- drivers/gpu/drm/panfrost/panfrost_gem.c | 3 ++- drivers/gpu/drm/panfrost/panfrost_mmu.c | 4 +++- drivers/gpu/drm/radeon/radeon_ttm.c | 11 ++++++----- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 15 +++++++------- drivers/gpu/drm/tegra/gem.c | 25 ++++++++++++------------ drivers/gpu/drm/tegra/plane.c | 13 ++++++------ drivers/gpu/drm/virtio/virtgpu_object.c | 11 ++++++----- drivers/gpu/drm/virtio/virtgpu_vq.c | 8 ++++---- drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 6 +++--- drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +- drivers/gpu/host1x/job.c | 17 ++++++++-------- drivers/media/pci/cx23885/cx23885-alsa.c | 2 +- drivers/media/pci/cx25821/cx25821-alsa.c | 2 +- drivers/media/pci/cx88/cx88-alsa.c | 2 +- drivers/media/pci/saa7134/saa7134-alsa.c | 2 +- drivers/media/platform/vsp1/vsp1_drm.c | 7 ++++--- drivers/staging/android/ion/ion.c | 17 ++++++++-------- drivers/staging/android/ion/ion_heap.c | 6 +++--- drivers/staging/android/ion/ion_system_heap.c | 2 +- drivers/xen/gntdev-dmabuf.c | 10 ++++++---- 35 files changed, 149 insertions(+), 121 deletions(-) -- 1.9.1

5 years, 1 month

2
24
0 0

Patch "dma-buf: Fix SET_NAME ioctl uapi" has been added to the 5.6-stable tree

by gregkh＠linuxfoundation.org

This is a note to let you know that I've just added the patch titled dma-buf: Fix SET_NAME ioctl uapi to the 5.6-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum… The filename of the patch is: dma-buf-fix-set_name-ioctl-uapi.patch and it can be found in the queue-5.6 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable(a)vger.kernel.org> know about it. >From a5bff92eaac45bdf6221badf9505c26792fdf99e Mon Sep 17 00:00:00 2001 From: Daniel Vetter <daniel.vetter(a)intel.com> Date: Tue, 7 Apr 2020 15:30:02 +0200 Subject: dma-buf: Fix SET_NAME ioctl uapi From: Daniel Vetter <daniel.vetter(a)intel.com> commit a5bff92eaac45bdf6221badf9505c26792fdf99e upstream. The uapi is the same on 32 and 64 bit, but the number isn't. Everyone who botched this please re-read: https://www.kernel.org/doc/html/v5.4-preprc-cpu/ioctl/botching-up-ioctls.ht… Also, the type argument for the ioctl macros is for the type the void __user *arg pointer points at, which in this case would be the variable-sized char[] of a 0 terminated string. So this was botched in more than just the usual ways. Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: Chenbo Feng <fengc(a)google.com> Cc: Greg Hackmann <ghackmann(a)google.com> Cc: Daniel Vetter <daniel.vetter(a)ffwll.ch> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: minchan(a)kernel.org Cc: surenb(a)google.com Cc: jenhaochen(a)google.com Cc: Martin Liu <liumartin(a)google.com> Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> Tested-by: Martin Liu <liumartin(a)google.com> Reviewed-by: Martin Liu <liumartin(a)google.com> Signed-off-by: Sumit Semwal <sumit.semwal(a)linaro.org> [sumits: updated some checkpatch fixes, corrected author email] Link: https://patchwork.freedesktop.org/patch/msgid/20200407133002.3486387-1-dani… Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/dma-buf/dma-buf.c | 3 ++- include/uapi/linux/dma-buf.h | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -388,7 +388,8 @@ static long dma_buf_ioctl(struct file *f return ret; - case DMA_BUF_SET_NAME: + case DMA_BUF_SET_NAME_A: + case DMA_BUF_SET_NAME_B: return dma_buf_set_name(dmabuf, (const char __user *)arg); default: --- a/include/uapi/linux/dma-buf.h +++ b/include/uapi/linux/dma-buf.h @@ -39,6 +39,12 @@ struct dma_buf_sync { #define DMA_BUF_BASE 'b' #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync) + +/* 32/64bitness of this uapi was botched in android, there's no difference + * between them in actual uapi, they're just different numbers. + */ #define DMA_BUF_SET_NAME _IOW(DMA_BUF_BASE, 1, const char *) +#define DMA_BUF_SET_NAME_A _IOW(DMA_BUF_BASE, 1, u32) +#define DMA_BUF_SET_NAME_B _IOW(DMA_BUF_BASE, 1, u64) #endif Patches currently in stable-queue which might be from daniel.vetter(a)intel.com are queue-5.6/dma-buf-fix-set_name-ioctl-uapi.patch

5 years, 1 month

1
0
0 0

Patch "dma-buf: Fix SET_NAME ioctl uapi" has been added to the 5.4-stable tree

by gregkh＠linuxfoundation.org

This is a note to let you know that I've just added the patch titled dma-buf: Fix SET_NAME ioctl uapi to the 5.4-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum… The filename of the patch is: dma-buf-fix-set_name-ioctl-uapi.patch and it can be found in the queue-5.4 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable(a)vger.kernel.org> know about it. >From a5bff92eaac45bdf6221badf9505c26792fdf99e Mon Sep 17 00:00:00 2001 From: Daniel Vetter <daniel.vetter(a)intel.com> Date: Tue, 7 Apr 2020 15:30:02 +0200 Subject: dma-buf: Fix SET_NAME ioctl uapi From: Daniel Vetter <daniel.vetter(a)intel.com> commit a5bff92eaac45bdf6221badf9505c26792fdf99e upstream. The uapi is the same on 32 and 64 bit, but the number isn't. Everyone who botched this please re-read: https://www.kernel.org/doc/html/v5.4-preprc-cpu/ioctl/botching-up-ioctls.ht… Also, the type argument for the ioctl macros is for the type the void __user *arg pointer points at, which in this case would be the variable-sized char[] of a 0 terminated string. So this was botched in more than just the usual ways. Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: Chenbo Feng <fengc(a)google.com> Cc: Greg Hackmann <ghackmann(a)google.com> Cc: Daniel Vetter <daniel.vetter(a)ffwll.ch> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: minchan(a)kernel.org Cc: surenb(a)google.com Cc: jenhaochen(a)google.com Cc: Martin Liu <liumartin(a)google.com> Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> Tested-by: Martin Liu <liumartin(a)google.com> Reviewed-by: Martin Liu <liumartin(a)google.com> Signed-off-by: Sumit Semwal <sumit.semwal(a)linaro.org> [sumits: updated some checkpatch fixes, corrected author email] Link: https://patchwork.freedesktop.org/patch/msgid/20200407133002.3486387-1-dani… Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/dma-buf/dma-buf.c | 3 ++- include/uapi/linux/dma-buf.h | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -388,7 +388,8 @@ static long dma_buf_ioctl(struct file *f return ret; - case DMA_BUF_SET_NAME: + case DMA_BUF_SET_NAME_A: + case DMA_BUF_SET_NAME_B: return dma_buf_set_name(dmabuf, (const char __user *)arg); default: --- a/include/uapi/linux/dma-buf.h +++ b/include/uapi/linux/dma-buf.h @@ -39,6 +39,12 @@ struct dma_buf_sync { #define DMA_BUF_BASE 'b' #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync) + +/* 32/64bitness of this uapi was botched in android, there's no difference + * between them in actual uapi, they're just different numbers. + */ #define DMA_BUF_SET_NAME _IOW(DMA_BUF_BASE, 1, const char *) +#define DMA_BUF_SET_NAME_A _IOW(DMA_BUF_BASE, 1, u32) +#define DMA_BUF_SET_NAME_B _IOW(DMA_BUF_BASE, 1, u64) #endif Patches currently in stable-queue which might be from daniel.vetter(a)intel.com are queue-5.4/dma-buf-fix-set_name-ioctl-uapi.patch

5 years, 1 month

1
0
0 0

Re: [Linaro-mm-sig] [PATCH] dma-buf: Documentation: fix: `make htmldocs` warnings

by Sam Ravnborg

On Wed, Apr 29, 2020 at 11:27:22PM -0300, Vitor Massaru Iha wrote: > On Wed, 2020-04-29 at 19:06 -0700, Randy Dunlap wrote: > > On 4/29/20 6:59 PM, Vitor Massaru Iha wrote: > > > Add missed ":" on kernel-doc function parameter. > > > > > > This patch fixes this warnings from `make htmldocs`: > > > ./drivers/dma-buf/dma-buf.c:678: warning: Function parameter or > > > member 'importer_ops' not described in 'dma_buf_dynamic_attach' > > > ./drivers/dma-buf/dma-buf.c:678: warning: Function parameter or > > > member 'importer_priv' not described in 'dma_buf_dynamic_attach' > > > > > > Signed-off-by: Vitor Massaru Iha <vitor(a)massaru.org> > > > --- > > > drivers/dma-buf/dma-buf.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > > > index ccc9eda1bc28..0756d2155745 100644 > > > --- a/drivers/dma-buf/dma-buf.c > > > +++ b/drivers/dma-buf/dma-buf.c > > > @@ -655,8 +655,8 @@ EXPORT_SYMBOL_GPL(dma_buf_put); > > > * calls attach() of dma_buf_ops to allow device-specific attach > > > functionality > > > * @dmabuf: [in] buffer to attach device to. > > > * @dev: [in] device to be attached. > > > - * @importer_ops [in] importer operations for the > > > attachment > > > - * @importer_priv [in] importer private pointer for the > > > attachment > > > + * @importer_ops: [in] importer operations for the > > > attachment > > > + * @importer_priv: [in] importer private pointer for the > > > attachment > > > * > > > * Returns struct dma_buf_attachment pointer for this attachment. > > > Attachments > > > * must be cleaned up by calling dma_buf_detach(). > > > > > > > Sumit said that he would be applying my patch from April 7: > > https://lore.kernel.org/linux-media/7bcbe6fe-0b4b-87da-d003-b68a26eb4cf0@in… > > > > thanks. > > Sorry. I didn't check if the patch has already been sent. Sumit - patch from Randy is neither applied to drm-misc-next nor drm-misc-fixes. A reminder in case it was lost somewhere. Sam

5 years, 1 month

2
1
0 0

Re: [Linaro-mm-sig] [Intel-gfx] [RFC 06/17] drm: i915: fix sg_table nents vs. orig_nents misuse

by Marek Szyprowski

Hi On 28.04.2020 16:27, Tvrtko Ursulin wrote: > > On 28/04/2020 14:19, Marek Szyprowski wrote: >> The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the >> numer of the created entries in the DMA address space. However the >> subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg >> must be >> called with the original number of entries passed to dma_map_sg. The >> sg_table->nents in turn holds the result of the dma_map_sg call as >> stated >> in include/linux/scatterlist.h. Adapt the code to obey those rules. >> >> Signed-off-by: Marek Szyprowski <m.szyprowski(a)samsung.com> >> --- >> drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 13 +++++++------ >> drivers/gpu/drm/i915/gem/i915_gem_internal.c | 4 ++-- >> drivers/gpu/drm/i915/gem/i915_gem_region.c | 4 ++-- >> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 5 +++-- >> drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 10 +++++----- >> drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c | 5 +++-- >> drivers/gpu/drm/i915/gt/intel_ggtt.c | 12 ++++++------ >> drivers/gpu/drm/i915/i915_gem_gtt.c | 12 +++++++----- >> drivers/gpu/drm/i915/i915_scatterlist.c | 4 ++-- >> drivers/gpu/drm/i915/selftests/scatterlist.c | 8 ++++---- >> 10 files changed, 41 insertions(+), 36 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c >> b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c >> index 7db5a79..d829852 100644 >> --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c >> +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c >> @@ -36,21 +36,22 @@ static struct sg_table >> *i915_gem_map_dma_buf(struct dma_buf_attachment *attachme >> goto err_unpin_pages; >> } >> - ret = sg_alloc_table(st, obj->mm.pages->nents, GFP_KERNEL); >> + ret = sg_alloc_table(st, obj->mm.pages->orig_nents, GFP_KERNEL); >> if (ret) >> goto err_free; >> src = obj->mm.pages->sgl; >> dst = st->sgl; >> - for (i = 0; i < obj->mm.pages->nents; i++) { >> + for (i = 0; i < obj->mm.pages->orig_nents; i++) { >> sg_set_page(dst, sg_page(src), src->length, 0); >> dst = sg_next(dst); >> src = sg_next(src); >> } >> - if (!dma_map_sg_attrs(attachment->dev, >> - st->sgl, st->nents, dir, >> - DMA_ATTR_SKIP_CPU_SYNC)) { >> + st->nents = dma_map_sg_attrs(attachment->dev, >> + st->sgl, st->orig_nents, dir, >> + DMA_ATTR_SKIP_CPU_SYNC); >> + if (!st->nents) { >> ret = -ENOMEM; >> goto err_free_sg; >> } >> @@ -74,7 +75,7 @@ static void i915_gem_unmap_dma_buf(struct >> dma_buf_attachment *attachment, >> struct drm_i915_gem_object *obj = >> dma_buf_to_obj(attachment->dmabuf); >> dma_unmap_sg_attrs(attachment->dev, >> - sg->sgl, sg->nents, dir, >> + sg->sgl, sg->orig_nents, dir, >> DMA_ATTR_SKIP_CPU_SYNC); >> sg_free_table(sg); >> kfree(sg); >> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_internal.c >> b/drivers/gpu/drm/i915/gem/i915_gem_internal.c >> index cbbff81..a8ebfdd 100644 >> --- a/drivers/gpu/drm/i915/gem/i915_gem_internal.c >> +++ b/drivers/gpu/drm/i915/gem/i915_gem_internal.c >> @@ -73,7 +73,7 @@ static int >> i915_gem_object_get_pages_internal(struct drm_i915_gem_object *obj) >> } >> sg = st->sgl; >> - st->nents = 0; >> + st->nents = st->orig_nents = 0; >> sg_page_sizes = 0; >> do { >> @@ -94,7 +94,7 @@ static int >> i915_gem_object_get_pages_internal(struct drm_i915_gem_object *obj) >> sg_set_page(sg, page, PAGE_SIZE << order, 0); >> sg_page_sizes |= PAGE_SIZE << order; >> - st->nents++; >> + st->nents = st->orig_nents = st->nents + 1; >> npages -= 1 << order; >> if (!npages) { >> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_region.c >> b/drivers/gpu/drm/i915/gem/i915_gem_region.c >> index 1515384..58ca560 100644 >> --- a/drivers/gpu/drm/i915/gem/i915_gem_region.c >> +++ b/drivers/gpu/drm/i915/gem/i915_gem_region.c >> @@ -53,7 +53,7 @@ >> GEM_BUG_ON(list_empty(blocks)); >> sg = st->sgl; >> - st->nents = 0; >> + st->nents = st->orig_nents = 0; >> sg_page_sizes = 0; >> prev_end = (resource_size_t)-1; >> @@ -78,7 +78,7 @@ >> sg->length = block_size; >> - st->nents++; >> + st->nents = st->orig_nents = st->nents + 1; >> } else { >> sg->length += block_size; >> sg_dma_len(sg) += block_size; >> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c >> b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c >> index 5d5d7ee..851a732 100644 >> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c >> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c >> @@ -80,7 +80,7 @@ static int shmem_get_pages(struct >> drm_i915_gem_object *obj) >> noreclaim |= __GFP_NORETRY | __GFP_NOWARN; >> sg = st->sgl; >> - st->nents = 0; >> + st->nents = st->orig_nents = 0; >> sg_page_sizes = 0; >> for (i = 0; i < page_count; i++) { >> const unsigned int shrink[] = { >> @@ -140,7 +140,8 @@ static int shmem_get_pages(struct >> drm_i915_gem_object *obj) >> sg_page_sizes |= sg->length; >> sg = sg_next(sg); >> } >> - st->nents++; >> + st->nents = st->orig_nents = st->nents + 1; > > A bit higher up, not shown in the patch, we have allocated a table via > sg_alloc_table giving it a pessimistic max nents, sometimes much > larger than the st->nents this loops will create. But orig_nents has > been now been overwritten. Will that leak memory come sg_table_free? Indeed this will leak memory. I'm missed that sg_trim() will adjust nents and orig_nents. I will drop those changes as they are only a creative (or hacky) way of using sg_table and scatterlists. > As minimum it will nerf our i915_sg_trim optimization a bit lower > down, also not shown in the diff. > > > [...] Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland

5 years, 1 month

1
0
0 0

Re: [Linaro-mm-sig] [PATCH] drm/radeon: drm/amdgpu: Disable [1002:6611] in radeon

by Alex Deucher

On Thu, Apr 30, 2020 at 9:08 AM Jian-Hong Pan <jian-hong(a)endlessm.com> wrote: > > The AMD/ATI Oland [1002:6611]'s HDMI output status is not synchronous > as shown on UI after hot re-plug the HDMI cable, if it is radeon in > used. The amdgpu module does not hit this issue. > > This patch disables [1002:6611] in radeon and enables it in amdgpu. > > Fixes: https://gitlab.freedesktop.org/drm/amd/-/issues/1117 > Signed-off-by: Jian-Hong Pan <jian-hong(a)endlessm.com> Nack. Amdgpu does not have support for VCE or UVD yet so you are just trading one issue for another. It would be better to figure out why the audio is not updated properly in some cases. Alex > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 11 +++++++++++ > include/drm/drm_pciids.h | 1 - > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > index 8ea86ffdea0d..1ad6f13a5bc0 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > @@ -1017,6 +1017,15 @@ MODULE_DEVICE_TABLE(pci, pciidlist); > > static struct drm_driver kms_driver; > > +static void amdgpu_pci_fixup(struct pci_dev *pdev) > +{ > +#ifdef CONFIG_DRM_AMDGPU_SI > + /* [1002:6611] is disabled in radeon, so enable si_support in amdgpu. */ > + if (pdev->vendor == PCI_VENDOR_ID_ATI && pdev->device == 0x6611) > + amdgpu_si_support = 1; > +#endif > +} > + > static int amdgpu_pci_probe(struct pci_dev *pdev, > const struct pci_device_id *ent) > { > @@ -1036,6 +1045,8 @@ static int amdgpu_pci_probe(struct pci_dev *pdev, > return -ENODEV; > } > > + amdgpu_pci_fixup(pdev); > + > #ifdef CONFIG_DRM_AMDGPU_SI > if (!amdgpu_si_support) { > switch (flags & AMD_ASIC_MASK) { > diff --git a/include/drm/drm_pciids.h b/include/drm/drm_pciids.h > index b7e899ce44f0..57368a0f5b82 100644 > --- a/include/drm/drm_pciids.h > +++ b/include/drm/drm_pciids.h > @@ -171,7 +171,6 @@ > {0x1002, 0x6607, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_OLAND|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \ > {0x1002, 0x6608, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_OLAND|RADEON_NEW_MEMMAP}, \ > {0x1002, 0x6610, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_OLAND|RADEON_NEW_MEMMAP}, \ > - {0x1002, 0x6611, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_OLAND|RADEON_NEW_MEMMAP}, \ > {0x1002, 0x6613, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_OLAND|RADEON_NEW_MEMMAP}, \ > {0x1002, 0x6617, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_OLAND|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \ > {0x1002, 0x6620, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_OLAND|RADEON_IS_MOBILITY|RADEON_NEW_MEMMAP}, \ > -- > 2.26.2 > > _______________________________________________ > amd-gfx mailing list > amd-gfx(a)lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx

5 years, 1 month

1
0
0 0

Re: [Linaro-mm-sig] [PATCH] dma-buf: Documentation: fix: `make htmldocs` warnings

by Randy Dunlap

On 4/29/20 6:59 PM, Vitor Massaru Iha wrote: > Add missed ":" on kernel-doc function parameter. > > This patch fixes this warnings from `make htmldocs`: > ./drivers/dma-buf/dma-buf.c:678: warning: Function parameter or member 'importer_ops' not described in 'dma_buf_dynamic_attach' > ./drivers/dma-buf/dma-buf.c:678: warning: Function parameter or member 'importer_priv' not described in 'dma_buf_dynamic_attach' > > Signed-off-by: Vitor Massaru Iha <vitor(a)massaru.org> > --- > drivers/dma-buf/dma-buf.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index ccc9eda1bc28..0756d2155745 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -655,8 +655,8 @@ EXPORT_SYMBOL_GPL(dma_buf_put); > * calls attach() of dma_buf_ops to allow device-specific attach functionality > * @dmabuf: [in] buffer to attach device to. > * @dev: [in] device to be attached. > - * @importer_ops [in] importer operations for the attachment > - * @importer_priv [in] importer private pointer for the attachment > + * @importer_ops: [in] importer operations for the attachment > + * @importer_priv: [in] importer private pointer for the attachment > * > * Returns struct dma_buf_attachment pointer for this attachment. Attachments > * must be cleaned up by calling dma_buf_detach(). > Sumit said that he would be applying my patch from April 7: https://lore.kernel.org/linux-media/7bcbe6fe-0b4b-87da-d003-b68a26eb4cf0@in… thanks. -- ~Randy

5 years, 1 month

1
0
0 0

[RFC 00/17] DRM: fix struct sg_table nents vs. orig_nents misuse

by Marek Szyprowski

Dear All, During the Exynos DRM GEM rework and fixing the issues in the drm_prime_sg_to_page_addr_arrays() function [1] I've noticed that most drivers in DRM framework incorrectly use nents and orig_nents entries of the struct sg_table. In case of the most DMA-mapping implementations exchanging those two entries or using nents for all loops on the scatterlist is harmless, because they both have the same value. There exists however a DMA-mapping implementations, for which such incorrect usage breaks things. The nents returned by dma_map_sg() might be lower than the nents passed as its parameter and this is perfectly fine. DMA framework or IOMMU is allowed to join consecutive chunks while mapping if such operation is supported by the underlying HW (bus, bridge, IOMMU, etc). Example of the case where dma_map_sg() might return 1 'DMA' chunk for the 4 'physical' pages is described here [2] The DMA-mapping framework documentation [3] states that dma_map_sg() returns the numer of the created entries in the DMA address space. However the subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be called with the original number of entries passed to dma_map_sg. The common pattern in DRM drivers were to assign the dma_map_sg() return value to sg_table->nents and use that value for the subsequent calls to dma_sync_sg_* or dma_unmap_sg functions. Also the code iterated over nents times to access the pages stored in the processed scatterlist, while it should use orig_nents as the numer of the page entries. I've tried to identify all such incorrect usage of sg_table->nents in the DRM subsystem and this is a result of my research. It looks that the incorrect pattern has been copied over the many drivers. Too bad in most cases it even worked correctly if the system used simple, linear DMA-mapping implementation, for which swapping nents and orig_nents doesn't make any difference. I wonder what to do to avoid such misuse in the future, as this case clearly shows that the current sg_table structure is a bit hard to understand. I have the following ideas and I would like to know your opinion before I prepare more patches and check sg_table usage all over the kernel: 1. introduce a dma_{map,sync,unmap}_sgtable() wrappers, which will use a proper sg_table entries and call respective DMA-mapping functions and adapt current code to it 2. rename nents and orig_nents to nr_pages, nr_dmas to clearly state which one refers to which part of the scatterlist; I'm open for other names for those entries I will appreciate your comments. Patches are based on top of Linux next-20200428. I've reduced the recipients list mainly to mailing lists, the next version I will try to send to the maintainers of the respective drivers. Best regards, Marek Szyprowski References: [1] https://lkml.org/lkml/2020/3/27/555 [2] https://lkml.org/lkml/2020/3/29/65 [3] Documentation/DMA-API-HOWTO.txt Patch summary: Marek Szyprowski (17): drm: core: fix sg_table nents vs. orig_nents usage drm: amdgpu: fix sg_table nents vs. orig_nents usage drm: armada: fix sg_table nents vs. orig_nents usage drm: etnaviv: fix sg_table nents vs. orig_nents usage drm: exynos: fix sg_table nents vs. orig_nents usage drm: i915: fix sg_table nents vs. orig_nents usage drm: lima: fix sg_table nents vs. orig_nents usage drm: msm: fix sg_table nents vs. orig_nents usage drm: panfrost: fix sg_table nents vs. orig_nents usage drm: radeon: fix sg_table nents vs. orig_nents usage drm: rockchip: fix sg_table nents vs. orig_nents usage drm: tegra: fix sg_table nents vs. orig_nents usage drm: virtio: fix sg_table nents vs. orig_nents usage drm: vmwgfx: fix sg_table nents vs. orig_nents usage drm: xen: fix sg_table nents vs. orig_nents usage drm: host1x: fix sg_table nents vs. orig_nents usage dmabuf: fix sg_table nents vs. orig_nents usage drivers/dma-buf/heaps/heap-helpers.c | 7 ++++--- drivers/dma-buf/udmabuf.c | 5 +++-- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 7 ++++--- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 ++++---- drivers/gpu/drm/armada/armada_gem.c | 14 ++++++++----- drivers/gpu/drm/drm_cache.c | 2 +- drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++--- drivers/gpu/drm/drm_prime.c | 9 +++++---- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 10 ++++++---- drivers/gpu/drm/exynos/exynos_drm_g2d.c | 7 ++++--- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 13 ++++++------ drivers/gpu/drm/i915/gem/i915_gem_internal.c | 4 ++-- drivers/gpu/drm/i915/gem/i915_gem_region.c | 4 ++-- drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 5 +++-- drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 10 +++++----- drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c | 5 +++-- drivers/gpu/drm/i915/gt/intel_ggtt.c | 12 ++++++------ drivers/gpu/drm/i915/i915_gem_gtt.c | 12 +++++++----- drivers/gpu/drm/i915/i915_scatterlist.c | 4 ++-- drivers/gpu/drm/i915/selftests/scatterlist.c | 8 ++++---- drivers/gpu/drm/lima/lima_gem.c | 4 ++-- drivers/gpu/drm/msm/msm_gem.c | 8 ++++---- drivers/gpu/drm/msm/msm_iommu.c | 3 ++- drivers/gpu/drm/panfrost/panfrost_gem.c | 3 ++- drivers/gpu/drm/panfrost/panfrost_mmu.c | 4 +++- drivers/gpu/drm/radeon/radeon_ttm.c | 10 +++++----- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 15 +++++++------- drivers/gpu/drm/tegra/gem.c | 25 ++++++++++++------------ drivers/gpu/drm/tegra/plane.c | 13 ++++++------ drivers/gpu/drm/virtio/virtgpu_object.c | 11 ++++++----- drivers/gpu/drm/virtio/virtgpu_vq.c | 8 ++++---- drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 6 +++--- drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +- drivers/gpu/host1x/job.c | 17 ++++++++-------- 34 files changed, 154 insertions(+), 128 deletions(-) -- 1.9.1

5 years, 1 month

5
22
0 0

[PATCH] dma-buf: Fix SET_NAME ioctl uapi

by Daniel Vetter

The uapi is the same on 32 and 64 bit, but the number isnt. Everyone who botched this please re-read: https://www.kernel.org/doc/html/v5.4-preprc-cpu/ioctl/botching-up-ioctls.ht… Also, the type argument for the ioctl macros is for the type the void __user *arg pointer points at, which in this case would be the variable-sized char[] of a 0 terminated string. So this was botched in more than just the usual ways. Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: Chenbo Feng <fengc(a)google.com> Cc: Greg Hackmann <ghackmann(a)google.com> Cc: Daniel Vetter <daniel.vetter(a)ffwll.ch> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: minchan(a)kernel.org Cc: surenb(a)google.com Cc: jenhaochen(a)google.com Cc: Martin Liu <liumartin(a)google.com> Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> --- drivers/dma-buf/dma-buf.c | 3 ++- include/uapi/linux/dma-buf.h | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 570c923023e6..1d923b8e4c59 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -388,7 +388,8 @@ static long dma_buf_ioctl(struct file *file, return ret; - case DMA_BUF_SET_NAME: + case DMA_BUF_SET_NAME_A: + case DMA_BUF_SET_NAME_B: return dma_buf_set_name(dmabuf, (const char __user *)arg); default: diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h index dbc7092e04b5..21dfac815dc0 100644 --- a/include/uapi/linux/dma-buf.h +++ b/include/uapi/linux/dma-buf.h @@ -39,6 +39,10 @@ struct dma_buf_sync { #define DMA_BUF_BASE 'b' #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync) +/* 32/64bitness of this uapi was botched in android, there's no difference + * between them in actual uapi, they're just different numbers. */ #define DMA_BUF_SET_NAME _IOW(DMA_BUF_BASE, 1, const char *) +#define DMA_BUF_SET_NAME_A _IOW(DMA_BUF_BASE, 1, u32) +#define DMA_BUF_SET_NAME_B _IOW(DMA_BUF_BASE, 1, u64) #endif -- 2.25.1

5 years, 1 month

3
3
0 0

Re: [Linaro-mm-sig] [PATCH] drm/amdgpu: remove conversion to bool in amdgpu_device.c

by Christian König

Am 27.04.20 um 08:36 schrieb Jason Yan: > The '>' expression itself is bool, no need to convert it to bool again. > This fixes the following coccicheck warning: > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:3004:68-73: WARNING: > conversion to bool not needed here > > Signed-off-by: Jason Yan <yanaijie(a)huawei.com> Reviewed-by: Christian König <christian.koenig(a)amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 3d601d5dd5af..ad94de3632d8 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -3000,7 +3000,7 @@ int amdgpu_device_init(struct amdgpu_device *adev, > INIT_WORK(&adev->xgmi_reset_work, amdgpu_device_xgmi_reset_func); > > adev->gfx.gfx_off_req_count = 1; > - adev->pm.ac_power = power_supply_is_system_supplied() > 0 ? true : false; > + adev->pm.ac_power = power_supply_is_system_supplied() > 0; > > /* Registers mapping */ > /* TODO: block userspace mapping of io register */

5 years, 1 month

2
1
0 0

Re: [Linaro-mm-sig] KASAN: use-after-free Read in vkms_dumb_create

by Hillf Danton

Sun, 26 Apr 2020 20:48:12 -0700 > syzbot found the following crash on: > > HEAD commit: c578ddb3 Merge tag 'linux-kselftest-5.7-rc3' of git://git... > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=10fbf0d8100000 > kernel config: https://syzkaller.appspot.com/x/.config?x=b7a70e992f2f9b68 > dashboard link: https://syzkaller.appspot.com/bug?extid=e3372a2afe1e7ef04bc7 > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15438330100000 > > Bisection is inconclusive: the first bad commit could be any of: > > 85b5bafb drm/cma-helper: Remove drm_fb_cma_fbdev_init_with_funcs() > dff1c703 drm/tinydrm: Use drm_fbdev_generic_setup() > 23167fa9 drm/panel: simple: Add support for Rocktech RK070ER9427 LCD panel > 9060d7f4 drm/fb-helper: Finish the generic fbdev emulation > 2230ca12 dt-bindings: display: Document the EDT et* displays in one file. > e896c132 drm/debugfs: Add internal client debugfs file > 894a677f drm/cma-helper: Use the generic fbdev emulation > aa7e6455 drm/panel: Add support for the EDT ETM0700G0BDH6 > 244007ec drm/pl111: Set .gem_prime_vmap and .gem_prime_mmap > aad34de2 drm/panel: Add support for the EDT ETM0700G0EDH6 > 7a6aca49 dt-bindings: Add vendor prefix for DLC Display Co., Ltd. > d536540f drm/fb-helper: Add generic fbdev emulation .fb_probe function > 0ca0c827 drm/panel: simple: Add DLC DLC0700YZG-1 panel > c76f0f7c drm: Begin an API for in-kernel clients > 5ba57bab drm: vkms: select DRM_KMS_HELPER > 5fa8e4a2 drm/panel: Make of_drm_find_panel() return an ERR_PTR() instead of NULL > 008095e0 drm/vc4: Add support for the transposer block > c59eb3cf drm/panel: Let of_drm_find_panel() return -ENODEV when the panel is disabled > 1ebe99a7 drm/vc4: Call drm_atomic_helper_fake_vblank() in the commit path > 2e64a174 drm/of: Make drm_of_find_panel_or_bridge() fail when the device is disabled > 1b9883ea drm/vc4: Support the case where the DSI device is disabled > 6fb42b66 drm/atomic: Call fake_vblank() from the generic commit_tail() helpers > b0b7aa40 dt-bindings: display: Add DT bindings for BOE HV070WSA-100 panel > b25c60af drm/crtc: Add a generic infrastructure to fake VBLANK events > 184d3cf4 drm/vc4: Use wait_for_flip_done() instead of wait_for_vblanks() > ae8cf41b drm/panel: simple: Add support for BOE HV070WSA-100 panel to simple-panel > 814bde99 drm/connector: Make ->atomic_commit() optional > 955f60db drm: Add support for extracting sync signal drive edge from videomode > 3b39ad7a drm/panel: simple: Add newhaven, nhd-4.3-480272ef-atxl LCD > 425132fd drm/connector: Pass a drm_connector_state to ->atomic_commit() > a5d2ade6 drm/panel: simple: Add support for Innolux G070Y2-L01 > b82c1f8f drm/atomic: Avoid connector to writeback_connector casts > 03fa9aa3 dt-bindings: Add DataImage, Inc. vendor prefix > 73915b2b drm/writeback: Fix the "overview" section of the doc > 97ceb1fb drm/panel: simple: Add support for DataImage SCF0700C48GGU18 > e22e9531 Merge drm-upstream/drm-next into drm-misc-next > 3d5664f9 drm/panel: ili9881c: Fix missing assignment to error return ret > a0120245 drm/crc: Only report a single overflow when a CRC fd is opened > 7ad4e463 drm/panel: p079zca: Refactor panel driver to support multiple panels > 8adbbb2e drm/stm: ltdc: rework reset sequence > 48bd379a drm/panel: p079zca: Add variable unprepare_delay properties > 7868e507 drm/stm: ltdc: filter mode pixel clock vs pad constraint > 731edd4c dt-bindings: Add Innolux P097PFG panel bindings > f8878bb2 drm: print plane state normalized zpos value > ca52bea9 drm/atomic-helper: Use bitwise or for filling a bitmask > de04a462 drm/panel: p079zca: Support Innolux P097PFG panel > 2bb7a39c dt-bindings: Add vendor prefix for kingdisplay > a65020d0 drm/v3d: Fix a grammar nit in the scheduler docs. > 2dd4f211 drm/v3d: Add missing v3d documentation structure. > ebc950fd dt-bindings: Add KINGDISPLAY KD097D04 panel bindings > cd0e0ca6 drm/panel: type promotion bug in s6e8aa0_read_mtp_id() > e0d01811 drm/v3d: Remove unnecessary dma_fence_ops. > 624bb0c0 drm/v3d: Delay the scheduler timeout if we're still making progress. > b6d83fcc drm/panel: p079zca: Use of_device_get_match_data() > 408633d2 drm/v3d: use new return type vm_fault_t in v3d_gem_fault > decac6b0 dt-bindings: display: sun4i-drm: Add R40 display engine compatible > 0b7510d1 drm/tilcdc: Use drm_connector_has_possible_encoder() > d978a94b drm/sun4i: Add R40 display engine compatible > af11942e drm/sun4i: tcon-top: Cleanup clock handling > f8222409 drm/msm: Use drm_connector_has_possible_encoder() > 38cb8d96 drm: Add drm_connector_has_possible_encoder() > da82107e drm/sun4i: tcon: Release node when traversing of graph > 7a667775 dt-bindings: display: sun4i-drm: Add R40 TV TCON description > 7b71ca24 drm/radeon: Use drm_connector_for_each_possible_encoder() > 4a068c5c drm/sun4i: DW HDMI: Release nodes if error happens during CRTC search > ddba766d drm/nouveau: Use drm_connector_for_each_possible_encoder() > 98c0e348 drm/amdgpu: Use drm_connector_for_each_possible_encoder() > e0f56782 drm/sun4i: mixer: Order includes alphabetically > 05db311a drm/sun4i: tcon-top: Add helpers for mux switching > 83aefbb8 drm: Add drm_connector_for_each_possible_encoder() > 20431c05 drm/i915: Nuke intel_mst_best_encoder() > 5e496566 drm/sun4i: tcon-top: Remove mux configuration at probe time > 0d998891 drm/fb-helper: Eliminate the .best_encoder() usage > ac1fe132 dt-bindings: display: sun4i-drm: Fix order of DW HDMI PHY compatibles > 03e3ec9a drm/panel: simple: Add Sharp LQ035Q7DB03 panel support > c91b007e drm/vkms: Add extra information about vkms > 5685ca0c drm/tinydrm: Fix doc build warnings > 854502fa drm/vkms: Add basic CRTC initialization > ae61f61f drm/client: Fix: drm_client_new: Don't require DRM to be registered > c04372ea drm/vkms: Add mode_config initialization > 41111ce1 drm/vkms: vkms_driver can be static > 559e50fd drm/vkms: Add dumb operations > 1c7c5fd9 drm/vkms: Introduce basic VKMS driver > 657cd71e drm: gma500: Changed __attribute__((packed)) to __packed > d1648930 drm/vkms: Add connectors helpers > > bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=17b65cdfe00000 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+e3372a2afe1e7ef04bc7(a)syzkaller.appspotmail.com > > ================================================================== > BUG: KASAN: use-after-free in vkms_dumb_create+0x286/0x290 drivers/gpu/drm/vkms/vkms_gem.c:142 > Read of size 8 at addr ffff88809e537110 by task syz-executor.0/9558 > > CPU: 0 PID: 9558 Comm: syz-executor.0 Not tainted 5.7.0-rc2-syzkaller #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dump_stack+0x188/0x20d lib/dump_stack.c:118 > print_address_description.constprop.0.cold+0xd3/0x315 mm/kasan/report.c:382 > __kasan_report.cold+0x35/0x4d mm/kasan/report.c:511 > kasan_report+0x33/0x50 mm/kasan/common.c:625 > vkms_dumb_create+0x286/0x290 drivers/gpu/drm/vkms/vkms_gem.c:142 > drm_mode_create_dumb+0x27c/0x300 drivers/gpu/drm/drm_dumb_buffers.c:94 > drm_ioctl_kernel+0x220/0x2f0 drivers/gpu/drm/drm_ioctl.c:787 > drm_ioctl+0x4c9/0x980 drivers/gpu/drm/drm_ioctl.c:887 > vfs_ioctl fs/ioctl.c:47 [inline] > ksys_ioctl+0x11a/0x180 fs/ioctl.c:763 > __do_sys_ioctl fs/ioctl.c:772 [inline] > __se_sys_ioctl fs/ioctl.c:770 [inline] > __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:770 > do_syscall_64+0xf6/0x7d0 arch/x86/entry/common.c:295 > entry_SYSCALL_64_after_hwframe+0x49/0xb3 > RIP: 0033:0x45c829 > Code: 0d b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 db b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00 > RSP: 002b:00007f19a3e30c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 > RAX: ffffffffffffffda RBX: 00000000004e2d80 RCX: 000000000045c829 > RDX: 0000000020000080 RSI: 00000000c02064b2 RDI: 0000000000000003 > RBP: 000000000078bf00 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff > R13: 000000000000028b R14: 00000000004d3188 R15: 00007f19a3e316d4 > > Allocated by task 9558: > save_stack+0x1b/0x40 mm/kasan/common.c:49 > set_track mm/kasan/common.c:57 [inline] > __kasan_kmalloc mm/kasan/common.c:495 [inline] > __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:468 > kmem_cache_alloc_trace+0x153/0x7d0 mm/slab.c:3551 > __vkms_gem_create+0x44/0xf0 include/linux/slab.h:555 > vkms_gem_create drivers/gpu/drm/vkms/vkms_gem.c:111 [inline] > vkms_gem_create drivers/gpu/drm/vkms/vkms_gem.c:100 [inline] > vkms_dumb_create+0x110/0x290 drivers/gpu/drm/vkms/vkms_gem.c:138 > drm_mode_create_dumb+0x27c/0x300 drivers/gpu/drm/drm_dumb_buffers.c:94 > drm_ioctl_kernel+0x220/0x2f0 drivers/gpu/drm/drm_ioctl.c:787 > drm_ioctl+0x4c9/0x980 drivers/gpu/drm/drm_ioctl.c:887 > vfs_ioctl fs/ioctl.c:47 [inline] > ksys_ioctl+0x11a/0x180 fs/ioctl.c:763 > __do_sys_ioctl fs/ioctl.c:772 [inline] > __se_sys_ioctl fs/ioctl.c:770 [inline] > __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:770 > do_syscall_64+0xf6/0x7d0 arch/x86/entry/common.c:295 > entry_SYSCALL_64_after_hwframe+0x49/0xb3 > > Freed by task 9558: > save_stack+0x1b/0x40 mm/kasan/common.c:49 > set_track mm/kasan/common.c:57 [inline] > kasan_set_free_info mm/kasan/common.c:317 [inline] > __kasan_slab_free+0xf7/0x140 mm/kasan/common.c:456 > __cache_free mm/slab.c:3426 [inline] > kfree+0x109/0x2b0 mm/slab.c:3757 > drm_gem_object_free+0xf0/0x1f0 drivers/gpu/drm/drm_gem.c:983 > kref_put include/linux/kref.h:65 [inline] > drm_gem_object_put_unlocked drivers/gpu/drm/drm_gem.c:1017 [inline] > drm_gem_object_put_unlocked+0x190/0x1c0 drivers/gpu/drm/drm_gem.c:1002 > vkms_gem_create drivers/gpu/drm/vkms/vkms_gem.c:116 [inline] > vkms_gem_create drivers/gpu/drm/vkms/vkms_gem.c:100 [inline] > vkms_dumb_create+0x14d/0x290 drivers/gpu/drm/vkms/vkms_gem.c:138 > drm_mode_create_dumb+0x27c/0x300 drivers/gpu/drm/drm_dumb_buffers.c:94 > drm_ioctl_kernel+0x220/0x2f0 drivers/gpu/drm/drm_ioctl.c:787 > drm_ioctl+0x4c9/0x980 drivers/gpu/drm/drm_ioctl.c:887 > vfs_ioctl fs/ioctl.c:47 [inline] > ksys_ioctl+0x11a/0x180 fs/ioctl.c:763 > __do_sys_ioctl fs/ioctl.c:772 [inline] > __se_sys_ioctl fs/ioctl.c:770 [inline] > __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:770 > do_syscall_64+0xf6/0x7d0 arch/x86/entry/common.c:295 > entry_SYSCALL_64_after_hwframe+0x49/0xb3 > > The buggy address belongs to the object at ffff88809e537000 > which belongs to the cache kmalloc-1k of size 1024 > The buggy address is located 272 bytes inside of > 1024-byte region [ffff88809e537000, ffff88809e537400) > The buggy address belongs to the page: > page:ffffea0002794dc0 refcount:1 mapcount:0 mapping:00000000e8234a18 index:0x0 > flags: 0xfffe0000000200(slab) > raw: 00fffe0000000200 ffffea00027a3608 ffffea0002749008 ffff8880aa000c40 > raw: 0000000000000000 ffff88809e537000 0000000100000002 0000000000000000 > page dumped because: kasan: bad access detected > > Memory state around the buggy address: > ffff88809e537000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > ffff88809e537080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > >ffff88809e537100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > ^ > ffff88809e537180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > ffff88809e537200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > ================================================================== Nothing to do if we're allowed to do nothing in the error case. --- a/drivers/gpu/drm/vkms/vkms_gem.c +++ b/drivers/gpu/drm/vkms/vkms_gem.c @@ -113,7 +113,6 @@ struct drm_gem_object *vkms_gem_create(s return ERR_CAST(obj); ret = drm_gem_handle_create(file, &obj->gem, handle); - drm_gem_object_put_unlocked(&obj->gem); if (ret) return ERR_PTR(ret);

5 years, 1 month

1
0
0 0

Re: [Linaro-mm-sig] [PATCH] misc: sram: Add dma-heap-export reserved SRAM area type

by John Stultz

On Fri, Apr 24, 2020 at 3:27 PM Andrew F. Davis <afd(a)ti.com> wrote: > This new export type exposes to userspace the SRAM area as a DMA-Heap, > this allows for allocations as DMA-BUFs that can be consumed by various > DMA-BUF supporting devices. > > Signed-off-by: Andrew F. Davis <afd(a)ti.com> Nice! Very excited to have the first new heap (that didn't come with the initial patchset)! Overall looks good! I don't have any comment on the SRAM side of things, but a few minor questions/nits below. > diff --git a/drivers/misc/sram-dma-heap.c b/drivers/misc/sram-dma-heap.c > new file mode 100644 > index 000000000000..38df0397f294 > --- /dev/null > +++ b/drivers/misc/sram-dma-heap.c > @@ -0,0 +1,243 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * SRAM DMA-Heap userspace exporter > + * > + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ > + * Andrew F. Davis <afd(a)ti.com> > + */ > + > +#include <linux/dma-mapping.h> > +#include <linux/err.h> > +#include <linux/genalloc.h> > +#include <linux/io.h> > +#include <linux/mm.h> > +#include <linux/scatterlist.h> > +#include <linux/slab.h> > +#include <linux/dma-buf.h> > +#include <linux/dma-heap.h> > + > +#include "sram.h" > + > +struct sram_dma_heap { > + struct dma_heap *heap; > + struct gen_pool *pool; > +}; > + > +struct sram_dma_heap_buffer { > + struct gen_pool *pool; > + struct list_head attachments; > + struct mutex attachments_lock; > + unsigned long len; > + void *vaddr; > + phys_addr_t paddr; > +}; > + > +struct dma_heap_attachment { > + struct device *dev; > + struct sg_table *table; > + struct list_head list; > +}; > + > +static int dma_heap_attach(struct dma_buf *dmabuf, > + struct dma_buf_attachment *attachment) > +{ > + struct sram_dma_heap_buffer *buffer = dmabuf->priv; > + struct dma_heap_attachment *a; > + struct sg_table *table; > + > + a = kzalloc(sizeof(*a), GFP_KERNEL); > + if (!a) > + return -ENOMEM; > + > + table = kmalloc(sizeof(*table), GFP_KERNEL); > + if (!table) { > + kfree(a); > + return -ENOMEM; > + } > + if (sg_alloc_table(table, 1, GFP_KERNEL)) { > + kfree(table); > + kfree(a); > + return -ENOMEM; > + } > + sg_set_page(table->sgl, pfn_to_page(PFN_DOWN(buffer->paddr)), buffer->len, 0); > + > + a->table = table; > + a->dev = attachment->dev; > + INIT_LIST_HEAD(&a->list); > + > + attachment->priv = a; > + > + mutex_lock(&buffer->attachments_lock); > + list_add(&a->list, &buffer->attachments); > + mutex_unlock(&buffer->attachments_lock); > + > + return 0; > +} > + > +static void dma_heap_detatch(struct dma_buf *dmabuf, > + struct dma_buf_attachment *attachment) > +{ > + struct sram_dma_heap_buffer *buffer = dmabuf->priv; > + struct dma_heap_attachment *a = attachment->priv; > + > + mutex_lock(&buffer->attachments_lock); > + list_del(&a->list); > + mutex_unlock(&buffer->attachments_lock); > + > + sg_free_table(a->table); > + kfree(a->table); > + kfree(a); > +} > + > +static struct sg_table *dma_heap_map_dma_buf(struct dma_buf_attachment *attachment, > + enum dma_data_direction direction) > +{ > + struct dma_heap_attachment *a = attachment->priv; > + struct sg_table *table = a->table; > + > + if (!dma_map_sg_attrs(attachment->dev, table->sgl, table->nents, > + direction, DMA_ATTR_SKIP_CPU_SYNC)) Might be nice to have a comment as to why you're using SKIP_CPU_SYNC and why it's safe. > + return ERR_PTR(-ENOMEM); > + > + return table; > +} > + > +static void dma_heap_unmap_dma_buf(struct dma_buf_attachment *attachment, > + struct sg_table *table, > + enum dma_data_direction direction) > +{ > + dma_unmap_sg_attrs(attachment->dev, table->sgl, table->nents, > + direction, DMA_ATTR_SKIP_CPU_SYNC); > +} > + > +static void dma_heap_dma_buf_release(struct dma_buf *dmabuf) > +{ > + struct sram_dma_heap_buffer *buffer = dmabuf->priv; > + > + gen_pool_free(buffer->pool, (unsigned long)buffer->vaddr, buffer->len); > + kfree(buffer); > +} > + > +static int dma_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma) > +{ > + struct sram_dma_heap_buffer *buffer = dmabuf->priv; > + int ret; > + > + /* SRAM mappings are not cached */ > + vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot); > + > + ret = vm_iomap_memory(vma, buffer->paddr, buffer->len); > + if (ret) > + pr_err("Could not map buffer to userspace\n"); > + > + return ret; > +} > + > +static void *dma_heap_vmap(struct dma_buf *dmabuf) > +{ > + struct sram_dma_heap_buffer *buffer = dmabuf->priv; > + > + return buffer->vaddr; > +} > + > +const struct dma_buf_ops sram_dma_heap_buf_ops = { > + .attach = dma_heap_attach, > + .detach = dma_heap_detatch, > + .map_dma_buf = dma_heap_map_dma_buf, > + .unmap_dma_buf = dma_heap_unmap_dma_buf, > + .release = dma_heap_dma_buf_release, > + .mmap = dma_heap_mmap, > + .vmap = dma_heap_vmap, > +}; No begin/end_cpu_access functions here? I'm guessing it's because you're always using SKIP_CPU_SYNC so it wouldn't do anything? A small comment in the code might help. > + > +static int sram_dma_heap_allocate(struct dma_heap *heap, > + unsigned long len, > + unsigned long fd_flags, > + unsigned long heap_flags) > +{ > + struct sram_dma_heap *sram_dma_heap = dma_heap_get_drvdata(heap); > + struct sram_dma_heap_buffer *buffer; > + > + DEFINE_DMA_BUF_EXPORT_INFO(exp_info); > + struct dma_buf *dmabuf; > + int ret; > + > + buffer = kzalloc(sizeof(*buffer), GFP_KERNEL); > + if (!buffer) > + return -ENOMEM; > + buffer->pool = sram_dma_heap->pool; > + INIT_LIST_HEAD(&buffer->attachments); > + mutex_init(&buffer->attachments_lock); > + buffer->len = len; > + > + buffer->vaddr = (void *)gen_pool_alloc(buffer->pool, buffer->len); > + if (!buffer->vaddr) { > + ret = -ENOMEM; > + goto free_buffer; > + } > + > + buffer->paddr = gen_pool_virt_to_phys(buffer->pool, (unsigned long)buffer->vaddr); > + if (buffer->paddr == -1) { > + ret = -ENOMEM; > + goto free_pool; > + } > + > + /* create the dmabuf */ > + exp_info.ops = &sram_dma_heap_buf_ops; > + exp_info.size = buffer->len; > + exp_info.flags = fd_flags; > + exp_info.priv = buffer; > + dmabuf = dma_buf_export(&exp_info); > + if (IS_ERR(dmabuf)) { > + ret = PTR_ERR(dmabuf); > + goto free_pool; > + } > + > + ret = dma_buf_fd(dmabuf, fd_flags); > + if (ret < 0) { > + dma_buf_put(dmabuf); > + /* just return, as put will call release and that will free */ > + return ret; > + } > + > + return ret; > + > +free_pool: > + gen_pool_free(buffer->pool, (unsigned long)buffer->vaddr, buffer->len); > +free_buffer: > + kfree(buffer); > + > + return ret; > +} > + > +static struct dma_heap_ops sram_dma_heap_ops = { > + .allocate = sram_dma_heap_allocate, > +}; > + > +int sram_dma_heap_export(struct sram_dev *sram, This is totally a bikeshed thing (feel free to ignore), but maybe sram_dma_heap_create() or _add() would be a better name to avoid folks mixing it up with the dmabuf exporter? > + struct sram_reserve *block, > + phys_addr_t start, > + struct sram_partition *part) > +{ > + struct sram_dma_heap *sram_dma_heap; > + struct dma_heap_export_info exp_info; > + > + dev_info(sram->dev, "Exporting SRAM pool '%s'\n", block->label); Again, shed issue: but for terminology consistency (at least in the dmabuf heaps space), maybe heap instead of pool? Thanks so much again for submitting this! -john

5 years, 1 month

1
0
0 0

Re: [Linaro-mm-sig] [PATCH] dma-buf: heaps: Initialize during core instead of subsys

by John Stultz

On Fri, Apr 24, 2020 at 3:18 PM Andrew F. Davis <afd(a)ti.com> wrote: > > Some clients of DMA-Heaps probe earlier than subsys_initcall(), this > can cause issues when these clients call dma_heap_add() before the > core DMA-Heaps framework has initialized. DMA-Heaps should initialize > during core startup to get ahead of all users. > > Signed-off-by: Andrew F. Davis <afd(a)ti.com> No objection from me right off. Acked-by: John Stultz <john.stultz(a)linaro.org> thanks -john

5 years, 1 month

1
0
0 0

decruft the vmalloc API v2

by Christoph Hellwig

Hi all, Peter noticed that with some dumb luck you can toast the kernel address space with exported vmalloc symbols. I used this as an opportunity to decruft the vmalloc.c API and make it much more systematic. This also removes any chance to create vmalloc mappings outside the designated areas or using executable permissions from modules. Besides that it removes more than 300 lines of code. A git tree is also available here: git://git.infradead.org/users/hch/misc.git sanitize-vmalloc-api.2 Gitweb: http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/sanitize-vm… Changes since v1: - implement pgprot_nx for arm64 (Mark Rutland) - fix a patch description - properly pass pgprot to vmap in ion - add a new patch to fix vmap() API misuse - fix a vmap argument in x86 - two more vmalloc cleanups - cleanup use of the unmap_kernel_range API - rename ioremap_pbh to ioremap_phb

5 years, 1 month

3
32
0 0

[PATCH] staging: android: ion: Skip sync if not mapped

by Ørjan Eide

Only sync the sg-list of an Ion dma-buf attachment when the attachment is actually mapped on the device. dma-bufs may be synced at any time. It can be reached from user space via DMA_BUF_IOCTL_SYNC, so there are no guarantees from callers on when syncs may be attempted, and dma_buf_end_cpu_access() and dma_buf_begin_cpu_access() may not be paired. Since the sg_list's dma_address isn't set up until the buffer is used on the device, and dma_map_sg() is called on it, the dma_address will be NULL if sync is attempted on the dma-buf before it's mapped on a device. Before v5.0 (commit 55897af63091 ("dma-direct: merge swiotlb_dma_ops into the dma_direct code")) this was a problem as the dma-api (at least the swiotlb_dma_ops on arm64) would use the potentially invalid dma_address. How that failed depended on how the device handled physical address 0. If 0 was a valid address to physical ram, that page would get flushed a lot, while the actual pages in the buffer would not get synced correctly. While if 0 is an invalid physical address it may cause a fault and trigger a crash. In v5.0 this was incidentally fixed by commit 55897af63091 ("dma-direct: merge swiotlb_dma_ops into the dma_direct code"), as this moved the dma-api to use the page pointer in the sg_list, and (for Ion buffers at least) this will always be valid if the sg_list exists at all. But, this issue is re-introduced in v5.3 with commit 449fa54d6815 ("dma-direct: correct the physical addr in dma_direct_sync_sg_for_cpu/device") moves the dma-api back to the old behaviour and picks the dma_address that may be invalid. dma-buf core doesn't ensure that the buffer is mapped on the device, and thus have a valid sg_list, before calling the exporter's begin_cpu_access. Signed-off-by: Ørjan Eide <orjan.eide(a)arm.com> --- drivers/staging/android/ion/ion.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) This seems to be part of a bigger issue where dma-buf exporters assume that their dma-buf begin_cpu_access and end_cpu_access callbacks have a certain guaranteed behavior, which isn't ensured by dma-buf core. This patch fixes this in ion only, but it also needs to be fixed for other exporters, either handled like this in each exporter, or in dma-buf core before calling into the exporters. diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 38b51eace4f9..7b752ba0cb6d 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -173,6 +173,7 @@ struct ion_dma_buf_attachment { struct device *dev; struct sg_table *table; struct list_head list; + bool mapped:1; }; static int ion_dma_buf_attach(struct dma_buf *dmabuf, @@ -195,6 +196,7 @@ static int ion_dma_buf_attach(struct dma_buf *dmabuf, a->table = table; a->dev = attachment->dev; INIT_LIST_HEAD(&a->list); + a->mapped = false; attachment->priv = a; @@ -231,6 +233,8 @@ static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment, direction)) return ERR_PTR(-ENOMEM); + a->mapped = true; + return table; } @@ -238,6 +242,10 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment, struct sg_table *table, enum dma_data_direction direction) { + struct ion_dma_buf_attachment *a = attachment->priv; + + a->mapped = false; + dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction); } @@ -297,6 +305,8 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, mutex_lock(&buffer->lock); list_for_each_entry(a, &buffer->attachments, list) { + if (!a->mapped) + continue; dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents, direction); } @@ -320,6 +330,8 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf, mutex_lock(&buffer->lock); list_for_each_entry(a, &buffer->attachments, list) { + if (!a->mapped) + continue; dma_sync_sg_for_device(a->dev, a->table->sgl, a->table->nents, direction); } -- 2.17.1 IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

5 years, 1 month

6
12
0 0

Re: [Linaro-mm-sig] [PATCH 10/28] mm: only allow page table mappings for built-in zsmalloc

by Minchan Kim

Hi Sergey, On Fri, Apr 10, 2020 at 11:38:45AM +0900, Sergey Senozhatsky wrote: > On (20/04/09 10:08), Minchan Kim wrote: > > > > Even though I don't know how many usecase we have using zsmalloc as > > > > module(I heard only once by dumb reason), it could affect existing > > > > users. Thus, please include concrete explanation in the patch to > > > > justify when the complain occurs. > > > > > > The justification is 'we can unexport functions that have no sane reason > > > of being exported in the first place'. > > > > > > The Changelog pretty much says that. > > > > Okay, I hope there is no affected user since this patch. > > If there are someone, they need to provide sane reason why they want > > to have zsmalloc as module. > > I'm one of those who use zsmalloc as a module - mainly because I use zram > as a compressing general purpose block device, not as a swap device. > I create zram0, mkfs, mount, checkout and compile code, once done - > umount, rmmod. This reduces the number of writes to SSD. Some people use > tmpfs, but zram device(-s) can be much larger in size. That's a niche use > case and I'm not against the patch. It doesn't mean we couldn't use zsmalloc as module any longer. It means we couldn't use zsmalloc as module with pgtable mapping whcih was little bit faster on microbenchmark in some architecutre(However, I usually temped to remove it since it had several problems). However, we could still use zsmalloc as module as copy way instead of pgtable mapping. Thus, if someone really want to rollback the feature, they should provide reasonable reason why it doesn't work for them. "A little fast" wouldn't be enough to exports deep internal to the module. Thanks.

5 years, 1 month

2
2
0 0

decruft the vmalloc API

by Christoph Hellwig

Hi all, Peter noticed that with some dumb luck you can toast the kernel address space with exported vmalloc symbols. I used this as an opportunity to decruft the vmalloc.c API and make it much more systematic. This also removes any chance to create vmalloc mappings outside the designated areas or using executable permissions from modules. Besides that it removes more than 300 lines of code. A git tree is also available here: git://git.infradead.org/users/hch/misc.git sanitize-vmalloc-api Gitweb: http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/sanitize-vm…

5 years, 1 month

10
50
0 0

[PATCH v2] staging: android: ion: use macro DEFINE_DEBUGFS_ATTRIBUTE to define debugfs fops

by R Veera Kumar

It is more clear to use DEFINE_DEBUGFS_ATTRIBUTE to define debugfs file operation rather than DEFINE_SIMPLE_ATTRIBUTE. Found using coccinelle. Signed-off-by: R Veera Kumar <vkor(a)vkten.in> --- Changes in v2: - Give correct explanation for patch - Adjust git commit tag and msg accordingly --- drivers/staging/android/ion/ion.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 38b51eace4f9..dbe4018a6f83 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -554,8 +554,8 @@ static int debug_shrink_get(void *data, u64 *val) return 0; } -DEFINE_SIMPLE_ATTRIBUTE(debug_shrink_fops, debug_shrink_get, - debug_shrink_set, "%llu\n"); +DEFINE_DEBUGFS_ATTRIBUTE(debug_shrink_fops, debug_shrink_get, + debug_shrink_set, "%llu\n"); void ion_device_add_heap(struct ion_heap *heap) { -- 2.20.1

5 years, 1 month

2
1
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig