Re: [PATCH] dmabuf: fix dmabuf file poll uaf issue

Hi TJ, Seems I have got answers from [1], where it is agreed upon epoll() is the source of issue. Thanks a lot for the discussion. [1] https://lore.kernel.org/lkml/2d631f0615918...@google.com/ Thanks Charan On 5/5/2024 9:50 PM, Charan Teja Kalla wrote: > Thanks T.J for the re

Re: [PATCH] dmabuf: fix dmabuf file poll uaf issue

Thanks T.J for the reply!! On 5/4/2024 4:43 AM, T.J. Mercier wrote: > It looks like a similar conclusion about epoll was reached at: > https://lore.kernel.org/all/a87d7ef8-2c59-4dc5-ba0a-b821d1eff...@amd.com/ > I am unaware of this discussion. Thanks... > I agree with Christian that it should

Re: [PATCH] dmabuf: fix dmabuf file poll uaf issue

Thanks Christian/TJ for the inputs!! On 4/18/2024 12:16 PM, Christian König wrote: > As far as I can see the EPOLL holds a reference to the files it > contains. So it is perfectly valid to add the file descriptor to EPOLL > and then close it. > > In this case the file won't be closed, but be kept

Re: [PATCH] dma-buf: fix dma_buf_export init order

Thanks Christian for this nice cleanup!! On 12/6/2022 8:42 PM, Christian König wrote: > @@ -638,10 +630,21 @@ struct dma_buf *dma_buf_export(const struct > dma_buf_export_info *exp_info) > if (!try_module_get(exp_info->owner)) > return ERR_PTR(-ENOENT); > > + file =

Re: [Linaro-mm-sig] Re: [PATCH] dma-buf: Fix possible UAF in dma_buf_export

Thanks Christian/TJ for all your inputs!! On 11/24/2022 6:25 PM, Christian König wrote: >>> I was already wondering why the order is this way. >>> >>> Why is dma_buf_stats_setup() needing the file in the first place? >> >> dmabuf->file will be used in dma_buf_stats_setup(), the >>

Re: [PATCH] dma-buf: Fix possible UAF in dma_buf_export

Thanks T.J and Christian for the inputs. On 11/19/2022 7:00 PM, Christian König wrote: >> >>     Yes, exactly that's the idea. >> >>     The only alternatives I can see would be to either move allocating >>     the >>     file and so completing the dma_buf initialization last again or just >>    

Re: [PATCH] dma-buf: Fix possible UAF in dma_buf_export

Sometime back Dan also reported the same issue[1] where I do mentioned that fput()-->dma_buf_file_release() will remove it from the list. But it seems that I failed to notice fput() here calls the dma_buf_file_release() asynchronously i.e. dmabuf that is accessed in the close path is already

Re: [bug report] dma-buf: call dma_buf_stats_setup after dmabuf is in valid list

++ Adding Christian On 5/16/2022 12:19 PM, Dan Carpenter wrote: > Hello Charan Teja Reddy, > > The patch ef3a6b70507a: "dma-buf: call dma_buf_stats_setup after > dmabuf is in valid list" from May 10, 2022, leads to the following > Smatch static checker warning: > >

[PATCH V3 RESEND] dma-buf: ensure unique directory name for dmabuf stats

than relying on the get_next_ino() for the same reason of avoiding the duplicate inodes[1]. [1] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/patch/?id=e809d5f0b5c912fe981dce738f3283b2010665f0 Signed-off-by: Charan Teja Kalla Cc: # 5.15.x+ Reviewed-by: Greg Kroah-Hartman --- Changes

Re: [PATCH V3] dma-buf: ensure unique directory name for dmabuf stats

On 5/13/2022 3:59 PM, Christian König wrote: > Am 13.05.22 um 12:18 schrieb Charan Teja Kalla: >> On 5/13/2022 3:41 PM, Greg KH wrote: >>>> Reported-by: kernel test robot >>> The trest robot did not say that the dmabuf stat name was being >>> duplicat

Re: [PATCH V3] dma-buf: ensure unique directory name for dmabuf stats

On 5/13/2022 3:41 PM, Greg KH wrote: >> Reported-by: kernel test robot > The trest robot did not say that the dmabuf stat name was being > duplicated, did it? > It reported a printk warning on V2[1]. Should we remove this on V3? @Christian: Could you please drop this tag while merging? [1]

[PATCH V3] dma-buf: ensure unique directory name for dmabuf stats

than relying on the get_next_ino() for the same reason of avoiding the duplicate inodes[1]. [1] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/patch/?id=e809d5f0b5c912fe981dce738f3283b2010665f0 Reported-by: kernel test robot Signed-off-by: Charan Teja Kalla --- Changes in V3: -- Used

Re: [PATCH V2] dmabuf: ensure unique directory name for dmabuf stats

Thanks Christian for the comments!! On 5/11/2022 12:33 PM, Christian König wrote: > >> The single number approach, generated by atomic, wouldn't break the >> uapi, but that number won't give any meaningful information especially >> when this is targeted just for debug purpose. And just 'inode'

Re: [PATCH V2] dmabuf: ensure unique directory name for dmabuf stats

Thanks Christian for the inputs!! On 5/10/2022 10:52 PM, Christian König wrote:      if (!dmabuf || !dmabuf->file)    return -EINVAL; @@ -192,7 +193,8 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf)      /* create the directory for buffer stats */   

Re: [PATCH V2] dmabuf: ensure unique directory name for dmabuf stats

On 5/10/2022 8:42 PM, Christian König wrote: >>    * The information in the interface can also be used to derive >> per-exporter >>    * statistics. The data from the interface can be gathered on error >> conditions >> @@ -172,6 +172,7 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf) >>   { >>   

[PATCH V2] dmabuf: ensure unique directory name for dmabuf stats

f_stats_setup() failed with -EEXIST as it is trying to create the same directory entry. To make the directory entry as unique, append the unique_id for every inode. With this change the stats directory entries will be in the format of: /sys/kernel/dmabuf/buffers/. Signed-off-by: Charan Teja Kalla ---

Re: [PATCH] dmabuf: ensure unique directory name for dmabuf stats

Thanks Christian for the inputs!! On 5/10/2022 5:05 PM, Christian König wrote: > >> And what's to keep the seconds field from also being the same? > > Well exporting two DMA-bufs with the same ino in the same nanosecond > should be basically impossible, but I would rather opt for using a 64bit

Re: [PATCH] dmabuf: ensure unique directory name for dmabuf stats

Thanks Greg for the inputs!! On 5/10/2022 4:30 PM, Greg KH wrote: >> The dmabuf file uses get_next_ino()(through dma_buf_getfile() -> >> alloc_anon_inode()) to get an inode number and uses the same as a >> directory name under /sys/kernel/dmabuf/buffers/. This directory is >> used to collect the

[PATCH] dmabuf: ensure unique directory name for dmabuf stats

f_stats_setup() failed with -EEXIST as it is trying to create the same directory entry. To make the directory entry as unique, append the inode creation time to the inode. With this change the stats directory entries will be in the format of: /sys/kernel/dmabuf/buffers/-. Signed-off-by: Char

Re: [PATCH] dma-buf: call dma_buf_stats_setup after dmabuf is in valid list

Hello Mercier, On 5/10/2022 3:19 AM, T.J. Mercier wrote: > On Mon, May 9, 2022 at 12:50 PM Charan Teja Kalla > wrote: >> From: Charan Teja Reddy >> >> When dma_buf_stats_setup() fails, it closes the dmabuf file which >> results into the calling of dma_bu

[PATCH] dma-buf: call dma_buf_stats_setup after dmabuf is in valid list

From: Charan Teja Reddy When dma_buf_stats_setup() fails, it closes the dmabuf file which results into the calling of dma_buf_file_release() where it does list_del(>list_node) with out first adding it to the proper list. This is resulting into panic in the below path:

Re: [PATCH v2] dmabuf: use spinlock to access dmabuf->name

Thanks Mike for the inputs. On 6/22/2020 5:10 PM, Ruhl, Michael J wrote: >> -Original Message- >> From: charante=codeaurora@mg.codeaurora.org >> On Behalf Of Charan Teja >> Kalla >> Sent: Monday, June 22, 2020 5:26 AM >> To: Ruhl, Michael J ; Sumit

Re: [PATCH v2] dmabuf: use spinlock to access dmabuf->name

Hello Mike, On 6/19/2020 7:11 PM, Ruhl, Michael J wrote: >> -Original Message- >> From: charante=codeaurora@mg.codeaurora.org >> On Behalf Of Charan Teja >> Kalla >> Sent: Friday, June 19, 2020 7:57 AM >> To: Sumit Semwal ; Ruhl, Michael J >>

[PATCH v2] dmabuf: use spinlock to access dmabuf->name

There exists a sleep-while-atomic bug while accessing the dmabuf->name under mutex in the dmabuffs_dname(). This is caused from the SELinux permissions checks on a process where it tries to validate the inherited files from fork() by traversing them through iterate_fd() (which traverse files under

Re: [PATCH] dmabuf: use spinlock to access dmabuf->name

On 6/17/2020 11:13 PM, Ruhl, Michael J wrote: >> -Original Message- >> From: charante=codeaurora@mg.codeaurora.org >> On Behalf Of Charan Teja >> Kalla >> Sent: Wednesday, June 17, 2020 2:29 AM >> To: Ruhl, Michael J ; Sumit Semwal >>

Re: [PATCH] dmabuf: use spinlock to access dmabuf->name

On 6/17/2020 1:51 PM, David Laight wrote: > From: Charan Teja Kalla >> Sent: 17 June 2020 07:29 > ... >>>> If name is freed you will copy garbage, but the only way >>>> for that to happen is that _set_name or _release have to be called >>>> at ju

Re: [PATCH] dmabuf: use spinlock to access dmabuf->name

Thanks Michael for the comments.. On 6/16/2020 7:29 PM, Ruhl, Michael J wrote: >> -Original Message- >> From: dri-devel On Behalf Of >> Ruhl, Michael J >> Sent: Tuesday, June 16, 2020 9:51 AM >> To: Charan Teja Kalla ; Sumit Semwal >> ; open list:D

Re: [PATCH v2] dma-buf: Move dma_buf_release() from fops to dentry_ops

Thanks Sumit for the fix. On 6/11/2020 5:14 PM, Sumit Semwal wrote: > Charan Teja reported a 'use-after-free' in dmabuffs_dname [1], which > happens if the dma_buf_release() is called while the userspace is > accessing the dma_buf pseudo fs's dmabuffs_dname() in another process, > and

[PATCH] dmabuf: use spinlock to access dmabuf->name

There exists a sleep-while-atomic bug while accessing the dmabuf->name under mutex in the dmabuffs_dname(). This is caused from the SELinux permissions checks on a process where it tries to validate the inherited files from fork() by traversing them through iterate_fd() (which traverse files under

Re: [PATCH v2] dma-buf: fix use-after-free in dmabuffs_dname

Thank you for the reply. On 5/13/2020 9:33 PM, Sumit Semwal wrote: > On Wed, 13 May 2020 at 21:16, Daniel Vetter wrote: >> >> On Wed, May 13, 2020 at 02:51:12PM +0200, Greg KH wrote: >>> On Wed, May 13, 2020 at 05:40:26PM +0530, Charan Teja Kalla wrote: >>>>

Re: [PATCH v2] dma-buf: fix use-after-free in dmabuffs_dname

Thank you Greg for the comments. On 5/12/2020 2:22 PM, Greg KH wrote: > On Fri, May 08, 2020 at 12:11:03PM +0530, Charan Teja Reddy wrote: >> The following race occurs while accessing the dmabuf object exported as >> file: >> P1 P2 >> dma_buf_release()

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

Thank you Greg for the comments. On 5/6/2020 2:30 PM, Greg KH wrote: 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

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

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()