diff mbox series

[v5,01/11] file: Export __receive_fd() to modules

Message ID 20210315053721.189-2-xieyongji@bytedance.com
State New
Headers show
Series Introduce VDUSE - vDPA Device in Userspace | expand

Commit Message

Yongji Xie March 15, 2021, 5:37 a.m. UTC
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(+)

Comments

Christoph Hellwig March 15, 2021, 9:08 a.m. UTC | #1
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.
Yongji Xie March 15, 2021, 9:46 a.m. UTC | #2
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
Christian Brauner March 15, 2021, 2:44 p.m. UTC | #3
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
Yongji Xie March 16, 2021, 2:48 a.m. UTC | #4
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
Christoph Hellwig March 25, 2021, 8:23 a.m. UTC | #5
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.
Yongji Xie March 25, 2021, 11:04 a.m. UTC | #6
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 mbox series

Patch

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)
 {