Re: [PATCH] [RFC] dma-buf: fix race condition between poll and close

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 23.04.24 um 21:13 schrieb Dmitry Antipov:
Syzbot has found the race condition where 'fput()' is in progress
when 'dma_buf_poll()' makes an attempt to hold the 'struct file'
with zero 'f_count'. So use explicit 'atomic_long_inc_not_zero()'
to detect such a case and cancel an undergoing poll activity with
EPOLLERR.

Well this is really interesting, you are the second person which comes up with this nonsense.

To repeat what I already said on the other thread: Calling dma_buf_poll() while fput() is in progress is illegal in the first place.

So there is nothing to fix in dma_buf_poll(), but rather to figure out who is incorrectly calling fput().

Regards,
Christian.


Reported-by: syzbot+5d4cb6b4409edfd18646@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=5d4cb6b4409edfd18646
Signed-off-by: Dmitry Antipov <dmantipov@xxxxxxxxx>
---
  drivers/dma-buf/dma-buf.c | 23 ++++++++++++++++++-----
  1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 8fe5aa67b167..39eb75d23219 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -266,8 +266,17 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
  		spin_unlock_irq(&dmabuf->poll.lock);
if (events & EPOLLOUT) {
-			/* Paired with fput in dma_buf_poll_cb */
-			get_file(dmabuf->file);
+			/*
+			 * Catch the case when fput() is in progress
+			 * (e.g. due to close() from another thread).
+			 * Otherwise the paired fput() will be issued
+			 * from dma_buf_poll_cb().
+			 */
+			if (unlikely(!atomic_long_inc_not_zero(&file->f_count))) {
+				events = EPOLLERR;
+				dcb->active = 0;
+				goto out;
+			}
if (!dma_buf_poll_add_cb(resv, true, dcb))
  				/* No callback queued, wake up any other waiters */
@@ -289,8 +298,12 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
  		spin_unlock_irq(&dmabuf->poll.lock);
if (events & EPOLLIN) {
-			/* Paired with fput in dma_buf_poll_cb */
-			get_file(dmabuf->file);
+			/* See above */
+			if (unlikely(!atomic_long_inc_not_zero(&file->f_count))) {
+				events = EPOLLERR;
+				dcb->active = 0;
+				goto out;
+			}
if (!dma_buf_poll_add_cb(resv, false, dcb))
  				/* No callback queued, wake up any other waiters */
@@ -299,7 +312,7 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
  				events &= ~EPOLLIN;
  		}
  	}
-
+out:
  	dma_resv_unlock(resv);
  	return events;
  }





[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux