Hello Baineng,

On Mon, 27 Jul 2026 at 08:56, 寿柏能 shoubaineng@gmail.com 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.

寿柏能 shoubaineng@gmail.com 于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 david.laight.linux@gmail.com 于2026年7月14日周二 22:33写道：

...

On Tue, 14 Jul 2026 21:38:07 +0800 寿柏能 shoubaineng@gmail.com 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 david.laight.linux@gmail.com 于2026年7月14日周二 21:14写道：

...

On Tue, 14 Jul 2026 19:46:53 +0800 Baineng Shou shoubaineng@gmail.com 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