On 11/27/24 9:57 AM, Jann Horn wrote: > Hi! > > In fs/bcachefs/fs-io-direct.c, "struct dio_write" contains a pointer > to an mm_struct. This pointer is grabbed in bch2_direct_write() > (without any kind of refcount increment), and used in > bch2_dio_write_continue() for kthread_use_mm()/kthread_unuse_mm() > which are used to enable userspace memory access from kthread context. > I believe kthread_use_mm()/kthread_unuse_mm() require that the caller > guarantees that the MM hasn't gone through exit_mmap() yet (normally > by holding an mmget() reference). > > If we reach this codepath via io_uring, do we have a guarantee that > the mm_struct that called bch2_direct_write() is still alive and > hasn't yet gone through exit_mmap() when it is accessed from > bch2_dio_write_continue()? > > I don't know the async direct I/O codepath particularly well, so I > cc'ed the uring maintainers, who probably know this better than me. I _think_ this is fine as-is, even if it does look dubious and bcachefs arguably should grab an mm ref for this just for safety to avoid future problems. The reason is that bcachefs doesn't set FMODE_NOWAIT, which means that on the io_uring side it cannot do non-blocking issue of requests. This is slower as it always punts to an io-wq thread, which shares the same mm. Hence if the request is alive, there's always a thread with the same mm alive as well. Now if FMODE_NOWAIT was set, then the original task could exit. I'd need to dig a bit deeper to verify that would always be safe and there's not a of time today with a few days off in the US looming, so I'll defer that to next week. It certainly would be fine with an mm ref grabbed. -- Jens Axboe
