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/0000000000002d631f0615918f1e@xxxxxxxxxx/ Thanks Charan On 5/5/2024 9:50 PM, Charan Teja Kalla wrote: > 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-b821d1effc72@xxxxxxx/ >> > I am unaware of this discussion. Thanks... > >> I agree with Christian that it should not be possible for the file to >> be freed while inside dma_buf_poll. Aside from causing problems in >> dma_buf_poll, ep_item_poll itself would have issues dereferencing the >> freed file pointer. >> > Not sure about my understanding: ep_item_poll() always call the ->poll() > interface with a stable 'struct file' because of ep->mtx. This lock > ensures that: > a) If eventpoll_release_file() get the ep->mtx first, ->poll() > corresponds to the epitem(target file) will never be called, because it > is removed from the rdlist. > > b) If ep_send_events() get the ep->mtx() first, ->poll() will get > called with a stable 'struct file', __but the refcount(->f_count) of a > file can be zero__. I am saying that this is stable because the 'struct > file' contents are still valid till we are in ->poll(). > > Can you/Christian help me with what I am missing here to say that > ->poll() is receiving stale 'struct file*', please? > > And, If you are convinced with above, I think, It should have been the > responsibility of ->poll() implementation to have taken refcount on a > file that is going to be still valid even after ->poll() exits. Incase > of dma_buf_poll() implementation, it took the refcount on a file that is > not going to be valid once the dma_buf_poll() exits(because of mentioned > race with the freeing of the 'struct file*'). > > So, in dma_buf_poll(), Should we be using atomic_long_inc_not_zero() > based implementation to take the refcount on a file? > > Thanks, > Charan
