Messages in this thread | | | Date | Wed, 7 Feb 2024 21:26:01 -0300 | Subject | Re: [PATCH v4 8/9] rust: file: add `DeferredFdCloser` | From | Martin Rodriguez Reboredo <> |
| |
On 2/2/24 07:55, Alice Ryhl wrote: > To close an fd from kernel space, we could call `ksys_close`. However, > if we do this to an fd that is held using `fdget`, then we may trigger a > use-after-free. Introduce a helper that can be used to close an fd even > if the fd is currently held with `fdget`. This is done by grabbing an > extra refcount to the file and dropping it in a task work once we return > to userspace. > > This is necessary for Rust Binder because otherwise the user might try > to have Binder close its fd for /dev/binder, which would cause problems > as this happens inside an ioctl on /dev/binder, and ioctls hold the fd > using `fdget`. > > Additional motivation can be found in commit 80cd795630d6 ("binder: fix > use-after-free due to ksys_close() during fdget()") and in the comments > on `binder_do_fd_close`. > > If there is some way to detect whether an fd is currently held with > `fdget`, then this could be optimized to skip the allocation and task > work when this is not the case. Another possible optimization would be > to combine several fds into a single task work, since this is used with > fd arrays that might hold several fds. > > That said, it might not be necessary to optimize it, because Rust Binder > has two ways to send fds: BINDER_TYPE_FD and BINDER_TYPE_FDA. With > BINDER_TYPE_FD, it is userspace's responsibility to close the fd, so > this mechanism is used only by BINDER_TYPE_FDA, but fd arrays are used > rarely these days. > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> > --- > [...]
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
| |