| Message ID | 20210315053721.189-2-xieyongji@bytedance.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce VDUSE - vDPA Device in Userspace | expand |
On Mon, Mar 15, 2021 at 01:37:11PM +0800, Xie Yongji wrote: > Export __receive_fd() so that some modules can use > it to pass file descriptor between processes. I really don't think any non-core code should do that, especilly not modular mere driver code.
On Mon, Mar 15, 2021 at 5:08 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Mon, Mar 15, 2021 at 01:37:11PM +0800, Xie Yongji wrote: > > Export __receive_fd() so that some modules can use > > it to pass file descriptor between processes. > > I really don't think any non-core code should do that, especilly not > modular mere driver code. Do you see any issue? Now I think we're able to do that with the help of get_unused_fd_flags() and fd_install() in modules. But we may miss some security stuff in this way. So I try to export __receive_fd() and use it instead. Thanks, Yongji
On Mon, Mar 15, 2021 at 05:46:43PM +0800, Yongji Xie wrote: > On Mon, Mar 15, 2021 at 5:08 PM Christoph Hellwig <hch@infradead.org> wrote: > > > > On Mon, Mar 15, 2021 at 01:37:11PM +0800, Xie Yongji wrote: > > > Export __receive_fd() so that some modules can use > > > it to pass file descriptor between processes. > > > > I really don't think any non-core code should do that, especilly not > > modular mere driver code. > > Do you see any issue? Now I think we're able to do that with the help > of get_unused_fd_flags() and fd_install() in modules. But we may miss > some security stuff in this way. So I try to export __receive_fd() and > use it instead. The __receive_fd() helper was added for core-kernel code only and we mainly did it for the seccomp notifier (and scm rights). The "__" prefix was intended to convey that message. And I agree with Christoph that we should probably keep it that way since __receive_fd() allows a few operations that no driver should probably do. I can see it being kinda ok to export a variant that really only receives and installs an fd, i.e. if we were to export what's currently available as an inline helper: static inline int receive_fd(struct file *file, unsigned int o_flags) but definitely none of the fd replacement stuff; that shold be off-limits. The seccomp notifier is the only codepath that should even think about fd replacement since it's about managing the syscalls of another task. Drivers swapping out fds doesn't sound like a good idea to me. Christian
On Mon, Mar 15, 2021 at 10:44 PM Christian Brauner <christian.brauner@canonical.com> wrote: > > On Mon, Mar 15, 2021 at 05:46:43PM +0800, Yongji Xie wrote: > > On Mon, Mar 15, 2021 at 5:08 PM Christoph Hellwig <hch@infradead.org> wrote: > > > > > > On Mon, Mar 15, 2021 at 01:37:11PM +0800, Xie Yongji wrote: > > > > Export __receive_fd() so that some modules can use > > > > it to pass file descriptor between processes. > > > > > > I really don't think any non-core code should do that, especilly not > > > modular mere driver code. > > > > Do you see any issue? Now I think we're able to do that with the help > > of get_unused_fd_flags() and fd_install() in modules. But we may miss > > some security stuff in this way. So I try to export __receive_fd() and > > use it instead. > > The __receive_fd() helper was added for core-kernel code only and we > mainly did it for the seccomp notifier (and scm rights). The "__" prefix > was intended to convey that message. > And I agree with Christoph that we should probably keep it that way > since __receive_fd() allows a few operations that no driver should > probably do. > I can see it being kinda ok to export a variant that really only > receives and installs an fd, i.e. if we were to export what's currently > available as an inline helper: > > static inline int receive_fd(struct file *file, unsigned int o_flags) > > but definitely none of the fd replacement stuff; that shold be > off-limits. The seccomp notifier is the only codepath that should even > think about fd replacement since it's about managing the syscalls of > another task. Drivers swapping out fds doesn't sound like a good idea to > me. > Thanks for the explanation, I got it. I will switch to use receive_fd() in the next version. Thanks, Yongji
On Mon, Mar 15, 2021 at 05:46:43PM +0800, Yongji Xie wrote: > On Mon, Mar 15, 2021 at 5:08 PM Christoph Hellwig <hch@infradead.org> wrote: > > > > On Mon, Mar 15, 2021 at 01:37:11PM +0800, Xie Yongji wrote: > > > Export __receive_fd() so that some modules can use > > > it to pass file descriptor between processes. > > > > I really don't think any non-core code should do that, especilly not > > modular mere driver code. > > Do you see any issue? Now I think we're able to do that with the help > of get_unused_fd_flags() and fd_install() in modules. But we may miss > some security stuff in this way. So I try to export __receive_fd() and > use it instead. The real problem is now what helper to use, but rather that random drivers should not just mess with the FD table like that.
On Thu, Mar 25, 2021 at 4:25 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Mon, Mar 15, 2021 at 05:46:43PM +0800, Yongji Xie wrote: > > On Mon, Mar 15, 2021 at 5:08 PM Christoph Hellwig <hch@infradead.org> wrote: > > > > > > On Mon, Mar 15, 2021 at 01:37:11PM +0800, Xie Yongji wrote: > > > > Export __receive_fd() so that some modules can use > > > > it to pass file descriptor between processes. > > > > > > I really don't think any non-core code should do that, especilly not > > > modular mere driver code. > > > > Do you see any issue? Now I think we're able to do that with the help > > of get_unused_fd_flags() and fd_install() in modules. But we may miss > > some security stuff in this way. So I try to export __receive_fd() and > > use it instead. > > The real problem is now what helper to use, but rather that random > drivers should not just mess with the FD table like that. I see. I will use receive_fd() instead that only receives and installs an fd. This is indeed needed in our cases. Thanks, Yongji
diff --git a/fs/file.c b/fs/file.c index dab120b71e44..a2e5bcae63ba 100644 --- a/fs/file.c +++ b/fs/file.c @@ -1107,6 +1107,7 @@ int __receive_fd(int fd, struct file *file, int __user *ufd, unsigned int o_flag __receive_sock(file); return new_fd; } +EXPORT_SYMBOL(__receive_fd); static int ksys_dup3(unsigned int oldfd, unsigned int newfd, int flags) {
Export __receive_fd() so that some modules can use it to pass file descriptor between processes. Signed-off-by: Xie Yongji <xieyongji@bytedance.com> --- fs/file.c | 1 + 1 file changed, 1 insertion(+)