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;
}
