Hi David The rollback here is the simplest possible case (put_unused_fd + dma_buf_put), not the complex sockopt scenario you describe.
Baineng David Laight <[email protected]> 于2026年7月14日周二 22:33写道： > On Tue, 14 Jul 2026 21:38:07 +0800 > 寿柏能 <[email protected]> wrote: > > > Hi David, > > > > Thanks for the feedback. > > > > The concern is not just about the EFAULT return — it's about the race > > window between fd_install() and copy_to_user(). Once fd_install() > > returns, the fd is immediately observable by other threads in the same > > process (via /proc/self/fd, SCM_RIGHTS, etc.), even before > > copy_to_user() has a chance to fail. The triggering condition is a > > deliberate mprotect() flip, not a corrupted heap. > > That is what makes doing the close wrong. > But that is a program aggressively trying to hit the timing window, > not a normal program that has managed to pass an invalid pointer. > The most likely reason for a real program passing an invalid pointer > is a corrupted heap (assuming the stupid coding errors are fixed). > > It is really no different from the sockopt code that receives > SCM_RIGHTS messages. > In that case once you've removed the FILE from the socket (or similar) > you really don't want to have to put it back because the write to the > sockopt buffer or length field fails. > The chance of correctly reverting the kernel state is small - and won't > be tested. > > David > > > > > The fix itself is small and follows the standard kernel idiom: > > get_unused_fd_flags() reserves the fd without publishing it, so the > > window between reservation and install is entirely under kernel control. > > > > Baineng > > > > David Laight <[email protected]> 于2026年7月14日周二 21:14写道： > > > > > On Tue, 14 Jul 2026 19:46:53 +0800 > > > Baineng Shou <[email protected]> wrote: > > > > > > > DMA_HEAP_IOCTL_ALLOC allocates a dma-buf and installs an fd into the > > > > caller's fd table via dma_buf_fd() -> fd_install() before > > > > dma_heap_ioctl() copies the result back to userspace. If the > trailing > > > > copy_to_user() fails, userspace never learns the fd number, but the > > > > fd (and the underlying dma-buf reference) are already visible to > > > > other threads in the same process and are leaked for the lifetime of > > > > the process. > > > > > > > > The obvious "close it on the failure path" fix is unsafe: once > > > > fd_install() has run, another thread can already dup() the fd, send > > > > it via SCM_RIGHTS, or close() it and let its number be reused, so a > > > > subsequent close_fd() from the ioctl path can operate on an unrelated > > > > file. This was pointed out by Christian König on v1 [1]. > > > ... > > > > > > My 2c: > > > > > > The other option is just to leave it as a 'problem for user space'. > > > No reasonable program is going to handle the EFAULT return by doing > > > anything other than exiting. > > > Even getting an EFAULT is really an indication that the application > > > is already in a real mess - most likely with a badly corrupted heap. > > > > > > Anything else leaves error recovery code in the kernel that is pretty > > > much never executed and open to a variety of bugs. > > > While the recovery here is probably ok, there are some sockopt calls > > > where it is all more complicated. > > > > > > David > > > > >
