Hello Baineng, On Mon, 27 Jul 2026 at 08:56, 寿柏能 <[email protected]> wrote: > > Ping. Any chance this can be picked up?
Thank you for the patch. While discussing with T J, we realised that adding a selftest for this would also be helpful. Could you please re-submit with a selftest added? > > Baineng Thanks and best regards, Sumit. > > 寿柏能 <[email protected]> 于2026年7月15日周三 10:04写道： >> >> 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 >>> > > >>>
