* [PATCH 0/2] Export receive_fd() to modules and do some cleanups @ 2021-04-01 9:09 Xie Yongji 2021-04-01 9:09 ` [PATCH 1/2] file: Export receive_fd() to modules Xie Yongji ` (3 more replies) 0 siblings, 4 replies; 29+ messages in thread From: Xie Yongji @ 2021-04-01 9:09 UTC (permalink / raw) To: christian.brauner, hch, gregkh, arve, tkjos, maco, joel, hridya, surenb, viro, sargun, keescook, jasowang Cc: devel, linux-fsdevel This series starts from Christian's comments on the series[1]. We'd like to export receive_fd() which can not only be used by our module in the series[1] but also allow further cleanups like patch 2 does. Now this series is based on Christoph's patch[2]. [1] https://lore.kernel.org/linux-fsdevel/20210331080519.172-1-xieyongji@bytedance.com/ [2] https://lore.kernel.org/linux-fsdevel/20210325082209.1067987-2-hch@lst.de Xie Yongji (2): file: Export receive_fd() to modules binder: Use receive_fd() to receive file from another process drivers/android/binder.c | 4 ++-- fs/file.c | 6 ++++++ include/linux/file.h | 7 +++---- 3 files changed, 11 insertions(+), 6 deletions(-) -- 2.11.0 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/2] file: Export receive_fd() to modules 2021-04-01 9:09 [PATCH 0/2] Export receive_fd() to modules and do some cleanups Xie Yongji @ 2021-04-01 9:09 ` Xie Yongji 2021-04-01 9:52 ` Greg KH 2021-04-01 9:09 ` [PATCH 2/2] binder: Use receive_fd() to receive file from another process Xie Yongji ` (2 subsequent siblings) 3 siblings, 1 reply; 29+ messages in thread From: Xie Yongji @ 2021-04-01 9:09 UTC (permalink / raw) To: christian.brauner, hch, gregkh, arve, tkjos, maco, joel, hridya, surenb, viro, sargun, keescook, jasowang Cc: devel, linux-fsdevel Export receive_fd() so that some modules can use it to pass file descriptor across processes without missing any security stuffs. Signed-off-by: Xie Yongji <xieyongji@bytedance.com> --- fs/file.c | 6 ++++++ include/linux/file.h | 7 +++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/fs/file.c b/fs/file.c index 56986e55befa..2a80c6c3e147 100644 --- a/fs/file.c +++ b/fs/file.c @@ -1107,6 +1107,12 @@ int receive_fd_replace(int new_fd, struct file *file, unsigned int o_flags) return new_fd; } +int receive_fd(struct file *file, unsigned int o_flags) +{ + return __receive_fd(file, NULL, o_flags); +} +EXPORT_SYMBOL(receive_fd); + static int ksys_dup3(unsigned int oldfd, unsigned int newfd, int flags) { int err = -EBADF; diff --git a/include/linux/file.h b/include/linux/file.h index 2de2e4613d7b..51e830b4fe3a 100644 --- a/include/linux/file.h +++ b/include/linux/file.h @@ -94,6 +94,9 @@ extern void fd_install(unsigned int fd, struct file *file); extern int __receive_fd(struct file *file, int __user *ufd, unsigned int o_flags); + +extern int receive_fd(struct file *file, unsigned int o_flags); + static inline int receive_fd_user(struct file *file, int __user *ufd, unsigned int o_flags) { @@ -101,10 +104,6 @@ static inline int receive_fd_user(struct file *file, int __user *ufd, return -EFAULT; return __receive_fd(file, ufd, o_flags); } -static inline int receive_fd(struct file *file, unsigned int o_flags) -{ - return __receive_fd(file, NULL, o_flags); -} int receive_fd_replace(int new_fd, struct file *file, unsigned int o_flags); extern void flush_delayed_fput(void); -- 2.11.0 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] file: Export receive_fd() to modules 2021-04-01 9:09 ` [PATCH 1/2] file: Export receive_fd() to modules Xie Yongji @ 2021-04-01 9:52 ` Greg KH 2021-04-01 10:24 ` Yongji Xie 0 siblings, 1 reply; 29+ messages in thread From: Greg KH @ 2021-04-01 9:52 UTC (permalink / raw) To: Xie Yongji Cc: christian.brauner, hch, arve, tkjos, maco, joel, hridya, surenb, viro, sargun, keescook, jasowang, devel, linux-fsdevel On Thu, Apr 01, 2021 at 05:09:31PM +0800, Xie Yongji wrote: > Export receive_fd() so that some modules can use > it to pass file descriptor across processes without > missing any security stuffs. > > Signed-off-by: Xie Yongji <xieyongji@bytedance.com> > --- > fs/file.c | 6 ++++++ > include/linux/file.h | 7 +++---- > 2 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/fs/file.c b/fs/file.c > index 56986e55befa..2a80c6c3e147 100644 > --- a/fs/file.c > +++ b/fs/file.c > @@ -1107,6 +1107,12 @@ int receive_fd_replace(int new_fd, struct file *file, unsigned int o_flags) > return new_fd; > } > > +int receive_fd(struct file *file, unsigned int o_flags) > +{ > + return __receive_fd(file, NULL, o_flags); > +} > +EXPORT_SYMBOL(receive_fd); What module uses this? And why not EXPORT_SYMBOL_GPL()? thanks, greg k-h ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Re: [PATCH 1/2] file: Export receive_fd() to modules 2021-04-01 9:52 ` Greg KH @ 2021-04-01 10:24 ` Yongji Xie 0 siblings, 0 replies; 29+ messages in thread From: Yongji Xie @ 2021-04-01 10:24 UTC (permalink / raw) To: Greg KH Cc: Christian Brauner, Christoph Hellwig, arve, tkjos, maco, joel, Hridya Valsaraju, Suren Baghdasaryan, viro, Sargun Dhillon, Kees Cook, Jason Wang, devel, linux-fsdevel On Thu, Apr 1, 2021 at 5:52 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Thu, Apr 01, 2021 at 05:09:31PM +0800, Xie Yongji wrote: > > Export receive_fd() so that some modules can use > > it to pass file descriptor across processes without > > missing any security stuffs. > > > > Signed-off-by: Xie Yongji <xieyongji@bytedance.com> > > --- > > fs/file.c | 6 ++++++ > > include/linux/file.h | 7 +++---- > > 2 files changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/fs/file.c b/fs/file.c > > index 56986e55befa..2a80c6c3e147 100644 > > --- a/fs/file.c > > +++ b/fs/file.c > > @@ -1107,6 +1107,12 @@ int receive_fd_replace(int new_fd, struct file *file, unsigned int o_flags) > > return new_fd; > > } > > > > +int receive_fd(struct file *file, unsigned int o_flags) > > +{ > > + return __receive_fd(file, NULL, o_flags); > > +} > > +EXPORT_SYMBOL(receive_fd); > > What module uses this? > Looks like now it will be only used by the module in my proposal: https://lore.kernel.org/linux-fsdevel/20210331080519.172-1-xieyongji@bytedance.com/ > And why not EXPORT_SYMBOL_GPL()? > My fault, sorry. Thanks, Yongji ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-01 9:09 [PATCH 0/2] Export receive_fd() to modules and do some cleanups Xie Yongji 2021-04-01 9:09 ` [PATCH 1/2] file: Export receive_fd() to modules Xie Yongji @ 2021-04-01 9:09 ` Xie Yongji 2021-04-01 9:54 ` Greg KH 2021-04-01 9:53 ` [PATCH 0/2] Export receive_fd() to modules and do some cleanups Greg KH 2021-04-01 10:20 ` Christian Brauner 3 siblings, 1 reply; 29+ messages in thread From: Xie Yongji @ 2021-04-01 9:09 UTC (permalink / raw) To: christian.brauner, hch, gregkh, arve, tkjos, maco, joel, hridya, surenb, viro, sargun, keescook, jasowang Cc: devel, linux-fsdevel Use receive_fd() to receive file from another process instead of combination of get_unused_fd_flags() and fd_install(). This simplifies the logic and also makes sure we don't miss any security stuff. Signed-off-by: Xie Yongji <xieyongji@bytedance.com> --- drivers/android/binder.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index c119736ca56a..080bcab7d632 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -3728,7 +3728,7 @@ static int binder_apply_fd_fixups(struct binder_proc *proc, int ret = 0; list_for_each_entry(fixup, &t->fd_fixups, fixup_entry) { - int fd = get_unused_fd_flags(O_CLOEXEC); + int fd = receive_fd(fixup->file, O_CLOEXEC); if (fd < 0) { binder_debug(BINDER_DEBUG_TRANSACTION, @@ -3741,7 +3741,7 @@ static int binder_apply_fd_fixups(struct binder_proc *proc, "fd fixup txn %d fd %d\n", t->debug_id, fd); trace_binder_transaction_fd_recv(t, fd, fixup->offset); - fd_install(fd, fixup->file); + fput(fixup->file); fixup->file = NULL; if (binder_alloc_copy_to_buffer(&proc->alloc, t->buffer, fixup->offset, &fd, -- 2.11.0 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-01 9:09 ` [PATCH 2/2] binder: Use receive_fd() to receive file from another process Xie Yongji @ 2021-04-01 9:54 ` Greg KH 2021-04-01 10:12 ` Yongji Xie 2021-04-01 10:40 ` Christian Brauner 0 siblings, 2 replies; 29+ messages in thread From: Greg KH @ 2021-04-01 9:54 UTC (permalink / raw) To: Xie Yongji Cc: christian.brauner, hch, arve, tkjos, maco, joel, hridya, surenb, viro, sargun, keescook, jasowang, devel, linux-fsdevel On Thu, Apr 01, 2021 at 05:09:32PM +0800, Xie Yongji wrote: > Use receive_fd() to receive file from another process instead of > combination of get_unused_fd_flags() and fd_install(). This simplifies > the logic and also makes sure we don't miss any security stuff. But no logic is simplified here, and nothing is "missed", so I do not understand this change at all. > > Signed-off-by: Xie Yongji <xieyongji@bytedance.com> > --- > drivers/android/binder.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c > index c119736ca56a..080bcab7d632 100644 > --- a/drivers/android/binder.c > +++ b/drivers/android/binder.c > @@ -3728,7 +3728,7 @@ static int binder_apply_fd_fixups(struct binder_proc *proc, > int ret = 0; > > list_for_each_entry(fixup, &t->fd_fixups, fixup_entry) { > - int fd = get_unused_fd_flags(O_CLOEXEC); > + int fd = receive_fd(fixup->file, O_CLOEXEC); Why 2 spaces? > > if (fd < 0) { > binder_debug(BINDER_DEBUG_TRANSACTION, > @@ -3741,7 +3741,7 @@ static int binder_apply_fd_fixups(struct binder_proc *proc, > "fd fixup txn %d fd %d\n", > t->debug_id, fd); > trace_binder_transaction_fd_recv(t, fd, fixup->offset); > - fd_install(fd, fixup->file); > + fput(fixup->file); Are you sure this is the same??? I d onot understand the need for this change at all, what is wrong with the existing code here? thanks, greg k-h ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-01 9:54 ` Greg KH @ 2021-04-01 10:12 ` Yongji Xie 2021-04-01 10:42 ` Greg KH 2021-04-01 10:40 ` Christian Brauner 1 sibling, 1 reply; 29+ messages in thread From: Yongji Xie @ 2021-04-01 10:12 UTC (permalink / raw) To: Greg KH Cc: Christian Brauner, Christoph Hellwig, arve, tkjos, maco, joel, Hridya Valsaraju, Suren Baghdasaryan, viro, Sargun Dhillon, Kees Cook, Jason Wang, devel, linux-fsdevel On Thu, Apr 1, 2021 at 5:54 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Thu, Apr 01, 2021 at 05:09:32PM +0800, Xie Yongji wrote: > > Use receive_fd() to receive file from another process instead of > > combination of get_unused_fd_flags() and fd_install(). This simplifies > > the logic and also makes sure we don't miss any security stuff. > > But no logic is simplified here, and nothing is "missed", so I do not > understand this change at all. > I noticed that we have security_binder_transfer_file() when we transfer some fds. I'm not sure whether we need something like security_file_receive() here? Thanks, Yongji ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-01 10:12 ` Yongji Xie @ 2021-04-01 10:42 ` Greg KH 2021-04-01 11:29 ` Yongji Xie 0 siblings, 1 reply; 29+ messages in thread From: Greg KH @ 2021-04-01 10:42 UTC (permalink / raw) To: Yongji Xie Cc: devel, tkjos, Kees Cook, Suren Baghdasaryan, Jason Wang, Sargun Dhillon, Christoph Hellwig, Hridya Valsaraju, arve, viro, joel, linux-fsdevel, Christian Brauner, maco On Thu, Apr 01, 2021 at 06:12:51PM +0800, Yongji Xie wrote: > On Thu, Apr 1, 2021 at 5:54 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > On Thu, Apr 01, 2021 at 05:09:32PM +0800, Xie Yongji wrote: > > > Use receive_fd() to receive file from another process instead of > > > combination of get_unused_fd_flags() and fd_install(). This simplifies > > > the logic and also makes sure we don't miss any security stuff. > > > > But no logic is simplified here, and nothing is "missed", so I do not > > understand this change at all. > > > > I noticed that we have security_binder_transfer_file() when we > transfer some fds. I'm not sure whether we need something like > security_file_receive() here? Why would you? And where is "here"? still confused, greg k-h ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Re: Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-01 10:42 ` Greg KH @ 2021-04-01 11:29 ` Yongji Xie 2021-04-01 11:33 ` Greg KH 0 siblings, 1 reply; 29+ messages in thread From: Yongji Xie @ 2021-04-01 11:29 UTC (permalink / raw) To: Greg KH Cc: devel, tkjos, Kees Cook, Suren Baghdasaryan, Jason Wang, Sargun Dhillon, Christoph Hellwig, Hridya Valsaraju, arve, viro, joel, linux-fsdevel, Christian Brauner, maco On Thu, Apr 1, 2021 at 6:42 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Thu, Apr 01, 2021 at 06:12:51PM +0800, Yongji Xie wrote: > > On Thu, Apr 1, 2021 at 5:54 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > On Thu, Apr 01, 2021 at 05:09:32PM +0800, Xie Yongji wrote: > > > > Use receive_fd() to receive file from another process instead of > > > > combination of get_unused_fd_flags() and fd_install(). This simplifies > > > > the logic and also makes sure we don't miss any security stuff. > > > > > > But no logic is simplified here, and nothing is "missed", so I do not > > > understand this change at all. > > > > > > > I noticed that we have security_binder_transfer_file() when we > > transfer some fds. I'm not sure whether we need something like > > security_file_receive() here? > > Why would you? And where is "here"? > > still confused, > I mean do we need to go through the file_receive seccomp notifier when we receive fd (use get_unused_fd_flags() + fd_install now) from another process in binder_apply_fd_fixups(). Thanks, Yongji ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Re: Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-01 11:29 ` Yongji Xie @ 2021-04-01 11:33 ` Greg KH 2021-04-01 12:28 ` Yongji Xie 0 siblings, 1 reply; 29+ messages in thread From: Greg KH @ 2021-04-01 11:33 UTC (permalink / raw) To: Yongji Xie Cc: devel, tkjos, Kees Cook, Suren Baghdasaryan, Jason Wang, Sargun Dhillon, Christoph Hellwig, Hridya Valsaraju, arve, viro, joel, linux-fsdevel, Christian Brauner, maco On Thu, Apr 01, 2021 at 07:29:45PM +0800, Yongji Xie wrote: > On Thu, Apr 1, 2021 at 6:42 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > On Thu, Apr 01, 2021 at 06:12:51PM +0800, Yongji Xie wrote: > > > On Thu, Apr 1, 2021 at 5:54 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > On Thu, Apr 01, 2021 at 05:09:32PM +0800, Xie Yongji wrote: > > > > > Use receive_fd() to receive file from another process instead of > > > > > combination of get_unused_fd_flags() and fd_install(). This simplifies > > > > > the logic and also makes sure we don't miss any security stuff. > > > > > > > > But no logic is simplified here, and nothing is "missed", so I do not > > > > understand this change at all. > > > > > > > > > > I noticed that we have security_binder_transfer_file() when we > > > transfer some fds. I'm not sure whether we need something like > > > security_file_receive() here? > > > > Why would you? And where is "here"? > > > > still confused, > > > > I mean do we need to go through the file_receive seccomp notifier when > we receive fd (use get_unused_fd_flags() + fd_install now) from > another process in binder_apply_fd_fixups(). Why? this is internal things, why does seccomp come into play here? ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Re: Re: Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-01 11:33 ` Greg KH @ 2021-04-01 12:28 ` Yongji Xie 2021-04-01 14:09 ` Greg KH 0 siblings, 1 reply; 29+ messages in thread From: Yongji Xie @ 2021-04-01 12:28 UTC (permalink / raw) To: Greg KH Cc: devel, tkjos, Kees Cook, Suren Baghdasaryan, Jason Wang, Sargun Dhillon, Christoph Hellwig, Hridya Valsaraju, arve, viro, joel, linux-fsdevel, Christian Brauner, maco On Thu, Apr 1, 2021 at 7:33 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Thu, Apr 01, 2021 at 07:29:45PM +0800, Yongji Xie wrote: > > On Thu, Apr 1, 2021 at 6:42 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > On Thu, Apr 01, 2021 at 06:12:51PM +0800, Yongji Xie wrote: > > > > On Thu, Apr 1, 2021 at 5:54 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > > > On Thu, Apr 01, 2021 at 05:09:32PM +0800, Xie Yongji wrote: > > > > > > Use receive_fd() to receive file from another process instead of > > > > > > combination of get_unused_fd_flags() and fd_install(). This simplifies > > > > > > the logic and also makes sure we don't miss any security stuff. > > > > > > > > > > But no logic is simplified here, and nothing is "missed", so I do not > > > > > understand this change at all. > > > > > > > > > > > > > I noticed that we have security_binder_transfer_file() when we > > > > transfer some fds. I'm not sure whether we need something like > > > > security_file_receive() here? > > > > > > Why would you? And where is "here"? > > > > > > still confused, > > > > > > > I mean do we need to go through the file_receive seccomp notifier when > > we receive fd (use get_unused_fd_flags() + fd_install now) from > > another process in binder_apply_fd_fixups(). > > Why? this is internal things, why does seccomp come into play here? > We already have security_binder_transfer_file() to control the sender process. So for the receiver process, do we need the seccomp too? Or do I miss something here? Thanks, Yongji ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Re: Re: Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-01 12:28 ` Yongji Xie @ 2021-04-01 14:09 ` Greg KH 2021-04-02 9:12 ` Kees Cook 0 siblings, 1 reply; 29+ messages in thread From: Greg KH @ 2021-04-01 14:09 UTC (permalink / raw) To: Yongji Xie Cc: devel, tkjos, Kees Cook, maco, Jason Wang, joel, Christoph Hellwig, Hridya Valsaraju, arve, viro, Sargun Dhillon, linux-fsdevel, Christian Brauner, Suren Baghdasaryan On Thu, Apr 01, 2021 at 08:28:02PM +0800, Yongji Xie wrote: > On Thu, Apr 1, 2021 at 7:33 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > On Thu, Apr 01, 2021 at 07:29:45PM +0800, Yongji Xie wrote: > > > On Thu, Apr 1, 2021 at 6:42 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > On Thu, Apr 01, 2021 at 06:12:51PM +0800, Yongji Xie wrote: > > > > > On Thu, Apr 1, 2021 at 5:54 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > > > > > On Thu, Apr 01, 2021 at 05:09:32PM +0800, Xie Yongji wrote: > > > > > > > Use receive_fd() to receive file from another process instead of > > > > > > > combination of get_unused_fd_flags() and fd_install(). This simplifies > > > > > > > the logic and also makes sure we don't miss any security stuff. > > > > > > > > > > > > But no logic is simplified here, and nothing is "missed", so I do not > > > > > > understand this change at all. > > > > > > > > > > > > > > > > I noticed that we have security_binder_transfer_file() when we > > > > > transfer some fds. I'm not sure whether we need something like > > > > > security_file_receive() here? > > > > > > > > Why would you? And where is "here"? > > > > > > > > still confused, > > > > > > > > > > I mean do we need to go through the file_receive seccomp notifier when > > > we receive fd (use get_unused_fd_flags() + fd_install now) from > > > another process in binder_apply_fd_fixups(). > > > > Why? this is internal things, why does seccomp come into play here? > > > > We already have security_binder_transfer_file() to control the sender > process. So for the receiver process, do we need the seccomp too? Or > do I miss something here? I do not know, is this something that is a requirement that seccomp handle all filesystem handles sent to a process? I do not know the seccomp "guarantee" that well, sorry. greg k-h ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Re: Re: Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-01 14:09 ` Greg KH @ 2021-04-02 9:12 ` Kees Cook 0 siblings, 0 replies; 29+ messages in thread From: Kees Cook @ 2021-04-02 9:12 UTC (permalink / raw) To: Greg KH Cc: Yongji Xie, devel, tkjos, maco, Jason Wang, joel, Christoph Hellwig, Hridya Valsaraju, arve, viro, Sargun Dhillon, linux-fsdevel, Christian Brauner, Suren Baghdasaryan On Thu, Apr 01, 2021 at 04:09:57PM +0200, Greg KH wrote: > On Thu, Apr 01, 2021 at 08:28:02PM +0800, Yongji Xie wrote: > > On Thu, Apr 1, 2021 at 7:33 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > On Thu, Apr 01, 2021 at 07:29:45PM +0800, Yongji Xie wrote: > > > > On Thu, Apr 1, 2021 at 6:42 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > > > On Thu, Apr 01, 2021 at 06:12:51PM +0800, Yongji Xie wrote: > > > > > > On Thu, Apr 1, 2021 at 5:54 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > > > > > > > On Thu, Apr 01, 2021 at 05:09:32PM +0800, Xie Yongji wrote: > > > > > > > > Use receive_fd() to receive file from another process instead of > > > > > > > > combination of get_unused_fd_flags() and fd_install(). This simplifies > > > > > > > > the logic and also makes sure we don't miss any security stuff. > > > > > > > > > > > > > > But no logic is simplified here, and nothing is "missed", so I do not > > > > > > > understand this change at all. > > > > > > > > > > > > > > > > > > > I noticed that we have security_binder_transfer_file() when we > > > > > > transfer some fds. I'm not sure whether we need something like > > > > > > security_file_receive() here? > > > > > > > > > > Why would you? And where is "here"? > > > > > > > > > > still confused, > > > > > > > > > > > > > I mean do we need to go through the file_receive seccomp notifier when > > > > we receive fd (use get_unused_fd_flags() + fd_install now) from > > > > another process in binder_apply_fd_fixups(). > > > > > > Why? this is internal things, why does seccomp come into play here? > > > > > > > We already have security_binder_transfer_file() to control the sender > > process. So for the receiver process, do we need the seccomp too? Or > > do I miss something here? > > I do not know, is this something that is a requirement that seccomp > handle all filesystem handles sent to a process? I do not know the > seccomp "guarantee" that well, sorry. This is an extremely confused thread. seccomp _uses_ the receive_fd() API. receive_fd() calls the security_file_receive() LSM hook. The security_binder_*() LSM hooks are different yet. Please, let's wait for Christian to clarify his idea first. -- Kees Cook ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-01 9:54 ` Greg KH 2021-04-01 10:12 ` Yongji Xie @ 2021-04-01 10:40 ` Christian Brauner 2021-04-01 11:11 ` Yongji Xie 2021-04-16 5:19 ` Al Viro 1 sibling, 2 replies; 29+ messages in thread From: Christian Brauner @ 2021-04-01 10:40 UTC (permalink / raw) To: Greg KH Cc: Xie Yongji, hch, arve, tkjos, maco, joel, hridya, surenb, viro, sargun, keescook, jasowang, devel, linux-fsdevel On Thu, Apr 01, 2021 at 11:54:45AM +0200, Greg KH wrote: > On Thu, Apr 01, 2021 at 05:09:32PM +0800, Xie Yongji wrote: > > Use receive_fd() to receive file from another process instead of > > combination of get_unused_fd_flags() and fd_install(). This simplifies > > the logic and also makes sure we don't miss any security stuff. > > But no logic is simplified here, and nothing is "missed", so I do not > understand this change at all. > > > > > Signed-off-by: Xie Yongji <xieyongji@bytedance.com> > > --- > > drivers/android/binder.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c > > index c119736ca56a..080bcab7d632 100644 > > --- a/drivers/android/binder.c > > +++ b/drivers/android/binder.c > > @@ -3728,7 +3728,7 @@ static int binder_apply_fd_fixups(struct binder_proc *proc, > > int ret = 0; > > > > list_for_each_entry(fixup, &t->fd_fixups, fixup_entry) { > > - int fd = get_unused_fd_flags(O_CLOEXEC); > > + int fd = receive_fd(fixup->file, O_CLOEXEC); > > Why 2 spaces? > > > > > if (fd < 0) { > > binder_debug(BINDER_DEBUG_TRANSACTION, > > @@ -3741,7 +3741,7 @@ static int binder_apply_fd_fixups(struct binder_proc *proc, > > "fd fixup txn %d fd %d\n", > > t->debug_id, fd); > > trace_binder_transaction_fd_recv(t, fd, fixup->offset); > > - fd_install(fd, fixup->file); > > + fput(fixup->file); > > Are you sure this is the same??? > > I d onot understand the need for this change at all, what is wrong with > the existing code here? I suggested something like this. Some time back we added receive_fd() for seccomp and SCM_RIGHTS to have a unified way of installing file descriptors including taking care of handling sockets and running security hooks. The helper also encompasses the whole get_unused_fd() + fd_install dance. My suggestion was to look at all the places were we currently open-code this in drivers/: drivers/android/binder.c: int fd = get_unused_fd_flags(O_CLOEXEC); drivers/char/tpm/tpm_vtpm_proxy.c: fd = get_unused_fd_flags(O_RDWR); drivers/dma-buf/dma-buf.c: fd = get_unused_fd_flags(flags); drivers/dma-buf/sw_sync.c: int fd = get_unused_fd_flags(O_CLOEXEC); drivers/dma-buf/sync_file.c: int fd = get_unused_fd_flags(O_CLOEXEC); drivers/gpio/gpiolib-cdev.c: fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC); drivers/gpio/gpiolib-cdev.c: fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC); drivers/gpio/gpiolib-cdev.c: fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC); drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c: fd = get_unused_fd_flags(O_CLOEXEC); drivers/gpu/drm/drm_atomic_uapi.c: fence_state->fd = get_unused_fd_flags(O_CLOEXEC); drivers/gpu/drm/drm_lease.c: fd = get_unused_fd_flags(cl->flags & (O_CLOEXEC | O_NONBLOCK)); drivers/gpu/drm/drm_syncobj.c: fd = get_unused_fd_flags(O_CLOEXEC); drivers/gpu/drm/drm_syncobj.c: int fd = get_unused_fd_flags(O_CLOEXEC); drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c: out_fence_fd = get_unused_fd_flags(O_CLOEXEC); drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c: out_fence_fd = get_unused_fd_flags(O_CLOEXEC); drivers/gpu/drm/msm/msm_gem_submit.c: out_fence_fd = get_unused_fd_flags(O_CLOEXEC); drivers/gpu/drm/virtio/virtgpu_ioctl.c: out_fence_fd = get_unused_fd_flags(O_CLOEXEC); drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c: out_fence_fd = get_unused_fd_flags(O_CLOEXEC); drivers/infiniband/core/rdma_core.c: new_fd = get_unused_fd_flags(O_CLOEXEC); drivers/media/mc/mc-request.c: fd = get_unused_fd_flags(O_CLOEXEC); drivers/misc/cxl/api.c: rc = get_unused_fd_flags(flags); drivers/scsi/cxlflash/ocxl_hw.c: rc = get_unused_fd_flags(flags); drivers/scsi/cxlflash/ocxl_hw.c: dev_err(dev, "%s: get_unused_fd_flags failed rc=%d\n", drivers/tty/pty.c: fd = get_unused_fd_flags(flags); drivers/vfio/vfio.c: ret = get_unused_fd_flags(O_CLOEXEC); drivers/virt/nitro_enclaves/ne_misc_dev.c: enclave_fd = get_unused_fd_flags(O_CLOEXEC); and see whether all of them can be switched to simply using receive_fd(). I did a completely untested rough sketch to illustrate what I meant by using binder and devpts Xie seems to have just picked those two. But the change is obviously only worth it if all or nearly all callers can be switched over without risk of regression. It would most likely simplify quite a few codepaths though especially in the error paths since we can get rid of put_unused_fd() cleanup. But it requires buy in from others obviously. Christian ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-01 10:40 ` Christian Brauner @ 2021-04-01 11:11 ` Yongji Xie 2021-04-16 5:19 ` Al Viro 1 sibling, 0 replies; 29+ messages in thread From: Yongji Xie @ 2021-04-01 11:11 UTC (permalink / raw) To: Christian Brauner Cc: Greg KH, Christoph Hellwig, arve, tkjos, maco, joel, hridya, surenb, viro, sargun, keescook, Jason Wang, devel, linux-fsdevel On Thu, Apr 1, 2021 at 6:40 PM Christian Brauner <christian.brauner@ubuntu.com> wrote: > > On Thu, Apr 01, 2021 at 11:54:45AM +0200, Greg KH wrote: > > On Thu, Apr 01, 2021 at 05:09:32PM +0800, Xie Yongji wrote: > > > Use receive_fd() to receive file from another process instead of > > > combination of get_unused_fd_flags() and fd_install(). This simplifies > > > the logic and also makes sure we don't miss any security stuff. > > > > But no logic is simplified here, and nothing is "missed", so I do not > > understand this change at all. > > > > > > > > Signed-off-by: Xie Yongji <xieyongji@bytedance.com> > > > --- > > > drivers/android/binder.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c > > > index c119736ca56a..080bcab7d632 100644 > > > --- a/drivers/android/binder.c > > > +++ b/drivers/android/binder.c > > > @@ -3728,7 +3728,7 @@ static int binder_apply_fd_fixups(struct binder_proc *proc, > > > int ret = 0; > > > > > > list_for_each_entry(fixup, &t->fd_fixups, fixup_entry) { > > > - int fd = get_unused_fd_flags(O_CLOEXEC); > > > + int fd = receive_fd(fixup->file, O_CLOEXEC); > > > > Why 2 spaces? > > > > > > > > if (fd < 0) { > > > binder_debug(BINDER_DEBUG_TRANSACTION, > > > @@ -3741,7 +3741,7 @@ static int binder_apply_fd_fixups(struct binder_proc *proc, > > > "fd fixup txn %d fd %d\n", > > > t->debug_id, fd); > > > trace_binder_transaction_fd_recv(t, fd, fixup->offset); > > > - fd_install(fd, fixup->file); > > > + fput(fixup->file); > > > > Are you sure this is the same??? > > > > I d onot understand the need for this change at all, what is wrong with > > the existing code here? > > I suggested something like this. > Some time back we added receive_fd() for seccomp and SCM_RIGHTS to have > a unified way of installing file descriptors including taking care of > handling sockets and running security hooks. The helper also encompasses > the whole get_unused_fd() + fd_install dance. > My suggestion was to look at all the places were we currently open-code > this in drivers/: > Sorry for not following your suggestion. Yes, I looked at those places. But I found that we will add security_file_receive() implicitly if we replace get_unused_fd() + fd_install() with receive_fd() for most drivers. Not sure if there will be some regression. So I only do that where I think security_file_receive() is needed, that is this patch. Although it looks like this is not a good idea now... Thanks, Yongji ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-01 10:40 ` Christian Brauner 2021-04-01 11:11 ` Yongji Xie @ 2021-04-16 5:19 ` Al Viro 2021-04-16 5:55 ` Al Viro 1 sibling, 1 reply; 29+ messages in thread From: Al Viro @ 2021-04-16 5:19 UTC (permalink / raw) To: Christian Brauner Cc: Greg KH, Xie Yongji, hch, arve, tkjos, maco, joel, hridya, surenb, sargun, keescook, jasowang, devel, linux-fsdevel On Thu, Apr 01, 2021 at 12:40:34PM +0200, Christian Brauner wrote: > My suggestion was to look at all the places were we currently open-code > this in drivers/: > > drivers/android/binder.c: int fd = get_unused_fd_flags(O_CLOEXEC); > drivers/char/tpm/tpm_vtpm_proxy.c: fd = get_unused_fd_flags(O_RDWR); > drivers/dma-buf/dma-buf.c: fd = get_unused_fd_flags(flags); > drivers/dma-buf/sw_sync.c: int fd = get_unused_fd_flags(O_CLOEXEC); > drivers/dma-buf/sync_file.c: int fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpio/gpiolib-cdev.c: fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC); > drivers/gpio/gpiolib-cdev.c: fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC); > drivers/gpio/gpiolib-cdev.c: fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC); > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c: fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpu/drm/drm_atomic_uapi.c: fence_state->fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpu/drm/drm_lease.c: fd = get_unused_fd_flags(cl->flags & (O_CLOEXEC | O_NONBLOCK)); > drivers/gpu/drm/drm_syncobj.c: fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpu/drm/drm_syncobj.c: int fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c: out_fence_fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c: out_fence_fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpu/drm/msm/msm_gem_submit.c: out_fence_fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpu/drm/virtio/virtgpu_ioctl.c: out_fence_fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c: out_fence_fd = get_unused_fd_flags(O_CLOEXEC); > drivers/infiniband/core/rdma_core.c: new_fd = get_unused_fd_flags(O_CLOEXEC); > drivers/media/mc/mc-request.c: fd = get_unused_fd_flags(O_CLOEXEC); > drivers/misc/cxl/api.c: rc = get_unused_fd_flags(flags); > drivers/scsi/cxlflash/ocxl_hw.c: rc = get_unused_fd_flags(flags); > drivers/scsi/cxlflash/ocxl_hw.c: dev_err(dev, "%s: get_unused_fd_flags failed rc=%d\n", > drivers/tty/pty.c: fd = get_unused_fd_flags(flags); > drivers/vfio/vfio.c: ret = get_unused_fd_flags(O_CLOEXEC); > drivers/virt/nitro_enclaves/ne_misc_dev.c: enclave_fd = get_unused_fd_flags(O_CLOEXEC); > > and see whether all of them can be switched to simply using > receive_fd(). I did a completely untested rough sketch to illustrate > what I meant by using binder and devpts Xie seems to have just picked > those two. But the change is obviously only worth it if all or nearly > all callers can be switched over without risk of regression. > It would most likely simplify quite a few codepaths though especially in > the error paths since we can get rid of put_unused_fd() cleanup. > > But it requires buy in from others obviously. Opening a file can have non-trivial side effects; reserving a descriptor can't. Moreover, if you look at the second hit in your list, you'll see that we do *NOT* want that combined thing there - fd_install() is completely irreversible, so we can't do that until we made sure the reply (struct vtpm_proxy_new_dev) has been successfully copied to userland. No, your receive_fd_user() does not cover that. There's a bunch of other cases like that - the next ones on your list are drivers/dma-buf/sw_sync.c and drivers/dma-buf/sync_file.c, etc. So I would consider receive_fd() as an attractive nuisance if it is ever suggested (and available) for wide use... ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-16 5:19 ` Al Viro @ 2021-04-16 5:55 ` Al Viro 2021-04-16 13:42 ` Christian Brauner 0 siblings, 1 reply; 29+ messages in thread From: Al Viro @ 2021-04-16 5:55 UTC (permalink / raw) To: Christian Brauner Cc: Greg KH, Xie Yongji, hch, arve, tkjos, maco, joel, hridya, surenb, sargun, keescook, jasowang, devel, linux-fsdevel On Fri, Apr 16, 2021 at 05:19:50AM +0000, Al Viro wrote: > On Thu, Apr 01, 2021 at 12:40:34PM +0200, Christian Brauner wrote: > > and see whether all of them can be switched to simply using > > receive_fd(). I did a completely untested rough sketch to illustrate > > what I meant by using binder and devpts Xie seems to have just picked > > those two. But the change is obviously only worth it if all or nearly > > all callers can be switched over without risk of regression. > > It would most likely simplify quite a few codepaths though especially in > > the error paths since we can get rid of put_unused_fd() cleanup. > > > > But it requires buy in from others obviously. > > Opening a file can have non-trivial side effects; reserving a descriptor > can't. Moreover, if you look at the second hit in your list, you'll see > that we do *NOT* want that combined thing there - fd_install() is > completely irreversible, so we can't do that until we made sure the > reply (struct vtpm_proxy_new_dev) has been successfully copied to > userland. No, your receive_fd_user() does not cover that. > > There's a bunch of other cases like that - the next ones on your list > are drivers/dma-buf/sw_sync.c and drivers/dma-buf/sync_file.c, etc. FWIW, pretty much all ioctls that return descriptor as part of a structure stored to user-supplied address tend to be that way; some don't have any other output fields (in which case they probably would've been better off with just passing the descriptor as return value of ioctl(2)). Those might be served by that receive_fd_user() helper; anything that has several outputs won't be. The same goes for anything that has hard-to-undo operations as part of what they need to do: reserve fd set file up do hard-to-undo stuff install into descriptor table is the only feasible order of operations - reservation can fail, so it must go before the hard-to-undo part and install into descriptor table can't be undone at all, so it must come last. Looks like e.g. drivers/virt/nitro_enclaves/ne_misc_dev.c case might be of that sort... ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-16 5:55 ` Al Viro @ 2021-04-16 13:42 ` Christian Brauner 2021-04-16 14:09 ` Al Viro 0 siblings, 1 reply; 29+ messages in thread From: Christian Brauner @ 2021-04-16 13:42 UTC (permalink / raw) To: Al Viro Cc: Greg KH, Xie Yongji, hch, arve, tkjos, maco, joel, hridya, surenb, sargun, keescook, jasowang, devel, linux-fsdevel On Fri, Apr 16, 2021 at 05:55:16AM +0000, Al Viro wrote: > On Fri, Apr 16, 2021 at 05:19:50AM +0000, Al Viro wrote: > > On Thu, Apr 01, 2021 at 12:40:34PM +0200, Christian Brauner wrote: > > > > and see whether all of them can be switched to simply using > > > receive_fd(). I did a completely untested rough sketch to illustrate > > > what I meant by using binder and devpts Xie seems to have just picked > > > those two. But the change is obviously only worth it if all or nearly > > > all callers can be switched over without risk of regression. > > > It would most likely simplify quite a few codepaths though especially in > > > the error paths since we can get rid of put_unused_fd() cleanup. > > > > > > But it requires buy in from others obviously. > > > > Opening a file can have non-trivial side effects; reserving a descriptor > > can't. Moreover, if you look at the second hit in your list, you'll see > > that we do *NOT* want that combined thing there - fd_install() is > > completely irreversible, so we can't do that until we made sure the > > reply (struct vtpm_proxy_new_dev) has been successfully copied to > > userland. No, your receive_fd_user() does not cover that. > > > > There's a bunch of other cases like that - the next ones on your list > > are drivers/dma-buf/sw_sync.c and drivers/dma-buf/sync_file.c, etc. > > FWIW, pretty much all ioctls that return descriptor as part of a structure > stored to user-supplied address tend to be that way; some don't have any > other output fields (in which case they probably would've been better off > with just passing the descriptor as return value of ioctl(2)). Those > might be served by that receive_fd_user() helper; anything that has several > outputs won't be. The same goes for anything that has hard-to-undo > operations as part of what they need to do: > reserve fd > set file up > do hard-to-undo stuff > install into descriptor table > is the only feasible order of operations - reservation can fail, so > it must go before the hard-to-undo part and install into descriptor > table can't be undone at all, so it must come last. Looks like > e.g. drivers/virt/nitro_enclaves/ne_misc_dev.c case might be of > that sort... If receive_fd() or your receive_fd_user() proposal can replace a chunk of open-coded places in modules where the split between reserving the file descriptor and installing it is pointless it's probably already worth it. Random example from io_uring where the file is already opened way before (which yes, isn't a module afaik but another place where we have that pattern): static int io_uring_install_fd(struct io_ring_ctx *ctx, struct file *file) { int ret, fd; fd = get_unused_fd_flags(O_RDWR | O_CLOEXEC); if (fd < 0) return fd; ret = io_uring_add_task_file(ctx); if (ret) { put_unused_fd(fd); return ret; } fd_install(fd, file); return fd; } A practical question also is whether the security receive hook thing actually belongs into the receive_fd() and receive_fd_user() helpers for the general case or whether that's just something that very few callers would want. But in any case I'm unlikely to have time on my hands to play with sm like this any time soon. Christian ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-16 13:42 ` Christian Brauner @ 2021-04-16 14:09 ` Al Viro 2021-04-16 15:13 ` Christian Brauner 0 siblings, 1 reply; 29+ messages in thread From: Al Viro @ 2021-04-16 14:09 UTC (permalink / raw) To: Christian Brauner Cc: Greg KH, Xie Yongji, hch, arve, tkjos, maco, joel, hridya, surenb, sargun, keescook, jasowang, devel, linux-fsdevel On Fri, Apr 16, 2021 at 03:42:52PM +0200, Christian Brauner wrote: > > > are drivers/dma-buf/sw_sync.c and drivers/dma-buf/sync_file.c, etc. > > > > FWIW, pretty much all ioctls that return descriptor as part of a structure > > stored to user-supplied address tend to be that way; some don't have any > > other output fields (in which case they probably would've been better off > > with just passing the descriptor as return value of ioctl(2)). Those > > might be served by that receive_fd_user() helper; anything that has several > > outputs won't be. The same goes for anything that has hard-to-undo > > operations as part of what they need to do: > > reserve fd > > set file up > > do hard-to-undo stuff > > install into descriptor table > > is the only feasible order of operations - reservation can fail, so > > it must go before the hard-to-undo part and install into descriptor > > table can't be undone at all, so it must come last. Looks like > > e.g. drivers/virt/nitro_enclaves/ne_misc_dev.c case might be of > > that sort... > > If receive_fd() or your receive_fd_user() proposal can replace a chunk My what proposal? The thing is currently in linux/file.h, put there by Kees half a year ago... > of open-coded places in modules where the split between reserving the > file descriptor and installing it is pointless it's probably already > worth it. A helper for use in some of the simplest cases, with big fat warnings not to touch if the things are not entirely trivial - sure, why not, same as we have anon_inode_getfd(). But that's a convenience helper, not a general purpose primitive. > Random example from io_uring where the file is already opened > way before (which yes, isn't a module afaik but another place where we > have that pattern): > > static int io_uring_install_fd(struct io_ring_ctx *ctx, struct file *file) > { > int ret, fd; > > fd = get_unused_fd_flags(O_RDWR | O_CLOEXEC); > if (fd < 0) > return fd; > > ret = io_uring_add_task_file(ctx); Huh? It's ret = io_uring_add_task_file(ctx, file); in the mainline and I don't see how that sucker could work without having file passed to it. > if (ret) { > put_unused_fd(fd); > return ret; > } > fd_install(fd, file); > return fd; > } ... and that's precisely the situation where we have something that is not obvious how to undo; look into io_uring_add_task_file()... We have three things to do: (1) reserve a descriptor, (2) io_uring_add_task_file(), (3) install the file. (1) and (2) may fail, (1) is trivial to undo, (2) might be not, (3) is impossible to undo. So I'd say that in this particular case io_uring is being perfectly reasonable... ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-16 14:09 ` Al Viro @ 2021-04-16 15:13 ` Christian Brauner 2021-04-16 15:35 ` Al Viro 0 siblings, 1 reply; 29+ messages in thread From: Christian Brauner @ 2021-04-16 15:13 UTC (permalink / raw) To: Al Viro Cc: Greg KH, Xie Yongji, hch, arve, tkjos, maco, joel, hridya, surenb, sargun, keescook, jasowang, devel, linux-fsdevel On Fri, Apr 16, 2021 at 02:09:35PM +0000, Al Viro wrote: > On Fri, Apr 16, 2021 at 03:42:52PM +0200, Christian Brauner wrote: > > > > are drivers/dma-buf/sw_sync.c and drivers/dma-buf/sync_file.c, etc. > > > > > > FWIW, pretty much all ioctls that return descriptor as part of a structure > > > stored to user-supplied address tend to be that way; some don't have any > > > other output fields (in which case they probably would've been better off > > > with just passing the descriptor as return value of ioctl(2)). Those > > > might be served by that receive_fd_user() helper; anything that has several > > > outputs won't be. The same goes for anything that has hard-to-undo > > > operations as part of what they need to do: > > > reserve fd > > > set file up > > > do hard-to-undo stuff > > > install into descriptor table > > > is the only feasible order of operations - reservation can fail, so > > > it must go before the hard-to-undo part and install into descriptor > > > table can't be undone at all, so it must come last. Looks like > > > e.g. drivers/virt/nitro_enclaves/ne_misc_dev.c case might be of > > > that sort... > > > > If receive_fd() or your receive_fd_user() proposal can replace a chunk > > My what proposal? The thing is currently in linux/file.h, put there > by Kees half a year ago... Yeah, I know. I was just referring to that line: > > > might be served by that receive_fd_user() helper; anything that has several I wasn't trying to imply you were the author or instigator of the api. > > > of open-coded places in modules where the split between reserving the > > file descriptor and installing it is pointless it's probably already > > worth it. > > A helper for use in some of the simplest cases, with big fat warnings > not to touch if the things are not entirely trivial - sure, why not, > same as we have anon_inode_getfd(). But that's a convenience helper, > not a general purpose primitive. > > > Random example from io_uring where the file is already opened > > way before (which yes, isn't a module afaik but another place where we > > have that pattern): > > > > static int io_uring_install_fd(struct io_ring_ctx *ctx, struct file *file) > > { > > int ret, fd; > > > > fd = get_unused_fd_flags(O_RDWR | O_CLOEXEC); > > if (fd < 0) > > return fd; > > > > ret = io_uring_add_task_file(ctx); > > Huh? It's > ret = io_uring_add_task_file(ctx, file); > in the mainline and I don't see how that sucker could work without having > file passed to it. My point here was more that the _file_ has already been opened _before_ that call to io_uring_add_task_file(). But any potential non-trivial side-effects of opening that file that you correctly pointed out in an earlier mail has already happened by that time. Granted there are more obvious examples, e.g. the binder stuff. int fd = get_unused_fd_flags(O_CLOEXEC); if (fd < 0) { binder_debug(BINDER_DEBUG_TRANSACTION, "failed fd fixup txn %d fd %d\n", t->debug_id, fd); ret = -ENOMEM; break; } binder_debug(BINDER_DEBUG_TRANSACTION, "fd fixup txn %d fd %d\n", t->debug_id, fd); trace_binder_transaction_fd_recv(t, fd, fixup->offset); fd_install(fd, fixup->file); fixup->file = NULL; if (binder_alloc_copy_to_buffer(&proc->alloc, t->buffer, fixup->offset, &fd, sizeof(u32))) { ret = -EINVAL; break; } ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-16 15:13 ` Christian Brauner @ 2021-04-16 15:35 ` Al Viro 2021-04-16 15:58 ` Christian Brauner 0 siblings, 1 reply; 29+ messages in thread From: Al Viro @ 2021-04-16 15:35 UTC (permalink / raw) To: Christian Brauner Cc: Greg KH, Xie Yongji, hch, arve, tkjos, maco, joel, hridya, surenb, sargun, keescook, jasowang, devel, linux-fsdevel On Fri, Apr 16, 2021 at 05:13:10PM +0200, Christian Brauner wrote: > My point here was more that the _file_ has already been opened _before_ > that call to io_uring_add_task_file(). But any potential non-trivial > side-effects of opening that file that you correctly pointed out in an > earlier mail has already happened by that time. The file comes from io_uring_get_file(), the entire thing is within the io_ring_ctx constructor and the only side effect there is ->ring_sock creation. And that stays until the io_ring_ctx is freed. I'm _not_ saying I like io_uring style in general, BTW - in particular, ->ring_sock->file handling is a kludge (as is too much of interation with AF_UNIX machinery there). But from side effects POV we are fine there. > Granted there are more > obvious examples, e.g. the binder stuff. > > int fd = get_unused_fd_flags(O_CLOEXEC); > > if (fd < 0) { > binder_debug(BINDER_DEBUG_TRANSACTION, > "failed fd fixup txn %d fd %d\n", > t->debug_id, fd); > ret = -ENOMEM; > break; > } > binder_debug(BINDER_DEBUG_TRANSACTION, > "fd fixup txn %d fd %d\n", > t->debug_id, fd); > trace_binder_transaction_fd_recv(t, fd, fixup->offset); > fd_install(fd, fixup->file); > fixup->file = NULL; > if (binder_alloc_copy_to_buffer(&proc->alloc, t->buffer, > fixup->offset, &fd, > sizeof(u32))) { > ret = -EINVAL; > break; > } ... and it's actually broken, since this /* All copies must be 32-bit aligned and 32-bit size */ if (!check_buffer(alloc, buffer, buffer_offset, bytes)) return -EINVAL; in binder_alloc_copy_to_buffer() should've been done *before* fd_install(). If anything, it's an example of the situation when fd_receive() would be wrong... ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-16 15:35 ` Al Viro @ 2021-04-16 15:58 ` Christian Brauner 2021-04-16 16:00 ` Christian Brauner 2021-04-16 17:30 ` Al Viro 0 siblings, 2 replies; 29+ messages in thread From: Christian Brauner @ 2021-04-16 15:58 UTC (permalink / raw) To: Al Viro Cc: Greg KH, Xie Yongji, hch, arve, tkjos, maco, joel, hridya, surenb, sargun, keescook, jasowang, devel, linux-fsdevel On Fri, Apr 16, 2021 at 03:35:59PM +0000, Al Viro wrote: > On Fri, Apr 16, 2021 at 05:13:10PM +0200, Christian Brauner wrote: > > > My point here was more that the _file_ has already been opened _before_ > > that call to io_uring_add_task_file(). But any potential non-trivial > > side-effects of opening that file that you correctly pointed out in an > > earlier mail has already happened by that time. > > The file comes from io_uring_get_file(), the entire thing is within the > io_ring_ctx constructor and the only side effect there is ->ring_sock > creation. And that stays until the io_ring_ctx is freed. I'm _not_ > saying I like io_uring style in general, BTW - in particular, > ->ring_sock->file handling is a kludge (as is too much of interation > with AF_UNIX machinery there). But from side effects POV we are fine > there. > > > Granted there are more > > obvious examples, e.g. the binder stuff. > > > > int fd = get_unused_fd_flags(O_CLOEXEC); > > > > if (fd < 0) { > > binder_debug(BINDER_DEBUG_TRANSACTION, > > "failed fd fixup txn %d fd %d\n", > > t->debug_id, fd); > > ret = -ENOMEM; > > break; > > } > > binder_debug(BINDER_DEBUG_TRANSACTION, > > "fd fixup txn %d fd %d\n", > > t->debug_id, fd); > > trace_binder_transaction_fd_recv(t, fd, fixup->offset); > > fd_install(fd, fixup->file); > > fixup->file = NULL; > > if (binder_alloc_copy_to_buffer(&proc->alloc, t->buffer, > > fixup->offset, &fd, > > sizeof(u32))) { > > ret = -EINVAL; > > break; > > } > > ... and it's actually broken, since this > /* All copies must be 32-bit aligned and 32-bit size */ > if (!check_buffer(alloc, buffer, buffer_offset, bytes)) > return -EINVAL; > in binder_alloc_copy_to_buffer() should've been done *before* > fd_install(). If anything, it's an example of the situation when > fd_receive() would be wrong... They could probably refactor this but I'm not sure why they'd bother. If they fail processing any of those files they end up aborting the whole transaction. (And the original code didn't check the error code btw.) ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-16 15:58 ` Christian Brauner @ 2021-04-16 16:00 ` Christian Brauner 2021-04-16 17:00 ` Al Viro 2021-04-16 17:30 ` Al Viro 1 sibling, 1 reply; 29+ messages in thread From: Christian Brauner @ 2021-04-16 16:00 UTC (permalink / raw) To: Al Viro Cc: Greg KH, Xie Yongji, hch, arve, tkjos, maco, joel, hridya, surenb, sargun, keescook, jasowang, devel, linux-fsdevel On Fri, Apr 16, 2021 at 05:58:25PM +0200, Christian Brauner wrote: > On Fri, Apr 16, 2021 at 03:35:59PM +0000, Al Viro wrote: > > On Fri, Apr 16, 2021 at 05:13:10PM +0200, Christian Brauner wrote: > > > > > My point here was more that the _file_ has already been opened _before_ > > > that call to io_uring_add_task_file(). But any potential non-trivial > > > side-effects of opening that file that you correctly pointed out in an > > > earlier mail has already happened by that time. > > > > The file comes from io_uring_get_file(), the entire thing is within the > > io_ring_ctx constructor and the only side effect there is ->ring_sock > > creation. And that stays until the io_ring_ctx is freed. I'm _not_ > > saying I like io_uring style in general, BTW - in particular, > > ->ring_sock->file handling is a kludge (as is too much of interation > > with AF_UNIX machinery there). But from side effects POV we are fine > > there. > > > > > Granted there are more > > > obvious examples, e.g. the binder stuff. > > > > > > int fd = get_unused_fd_flags(O_CLOEXEC); > > > > > > if (fd < 0) { > > > binder_debug(BINDER_DEBUG_TRANSACTION, > > > "failed fd fixup txn %d fd %d\n", > > > t->debug_id, fd); > > > ret = -ENOMEM; > > > break; > > > } > > > binder_debug(BINDER_DEBUG_TRANSACTION, > > > "fd fixup txn %d fd %d\n", > > > t->debug_id, fd); > > > trace_binder_transaction_fd_recv(t, fd, fixup->offset); > > > fd_install(fd, fixup->file); > > > fixup->file = NULL; > > > if (binder_alloc_copy_to_buffer(&proc->alloc, t->buffer, > > > fixup->offset, &fd, > > > sizeof(u32))) { > > > ret = -EINVAL; > > > break; > > > } > > > > ... and it's actually broken, since this > > /* All copies must be 32-bit aligned and 32-bit size */ > > if (!check_buffer(alloc, buffer, buffer_offset, bytes)) > > return -EINVAL; > > in binder_alloc_copy_to_buffer() should've been done *before* > > fd_install(). If anything, it's an example of the situation when > > fd_receive() would be wrong... > > They could probably refactor this but I'm not sure why they'd bother. If > they fail processing any of those files they end up aborting the > whole transaction. > (And the original code didn't check the error code btw.) (dma_buf_fd() seems like another good candidate. But again, I don't have any plans to shove this down anyone's throat.) ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-16 16:00 ` Christian Brauner @ 2021-04-16 17:00 ` Al Viro 0 siblings, 0 replies; 29+ messages in thread From: Al Viro @ 2021-04-16 17:00 UTC (permalink / raw) To: Christian Brauner Cc: Greg KH, Xie Yongji, hch, arve, tkjos, maco, joel, hridya, surenb, sargun, keescook, jasowang, devel, linux-fsdevel On Fri, Apr 16, 2021 at 06:00:38PM +0200, Christian Brauner wrote: > (dma_buf_fd() seems like another good candidate. But again, I don't have > any plans to shove this down anyone's throat.) Sure, there are candidates for such a helper. Just as there are legitimate users of anon_inode_getfd(). However, it is *NOT* something we can use as a vehicle for some hooks (pardon the obscenity); it won't be consistently used in all cases - it simply is not feasible for many of the fd_install() users. Incidentally, looking at the user of receive_fd_user(), I would say that it would be easier to follow in this form: case VDUSE_IOTLB_GET_ENTRY: { struct vduse_iotlb_entry entry; struct vhost_iotlb_map *map; struct vduse_iova_domain *domain = dev->domain; struct file *f = NULL; if (copy_from_user(&entry, argp, sizeof(entry))) return -EFAULT; entry.fd = get_unused_fd_flags(perm_to_file_flags(entry.perm)); if (entry.fd < 0) return entry.fd; spin_lock(&domain->iotlb_lock); map = vhost_iotlb_itree_first(domain->iotlb, entry.start, entry.start + 1); if (map) { struct vdpa_map_file *map_file = map->opaque; f = get_file(map_file->file); entry.offset = map_file->offset; entry.start = map->start; entry.last = map->last; entry.perm = map->perm; } spin_unlock(&domain->iotlb_lock); if (!f) { put_unused_fd(entry.fd); return -EINVAL; } if (copy_to_user(argp, &entry, sizeof(entry))) { put_unused_fd(entry.fd); fput(f); return -EFAULT; } // point of no return fd_install(entry.fd, f); return 0; } ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-16 15:58 ` Christian Brauner 2021-04-16 16:00 ` Christian Brauner @ 2021-04-16 17:30 ` Al Viro 2021-04-17 1:30 ` Al Viro 1 sibling, 1 reply; 29+ messages in thread From: Al Viro @ 2021-04-16 17:30 UTC (permalink / raw) To: Christian Brauner Cc: Greg KH, Xie Yongji, hch, arve, tkjos, maco, joel, hridya, surenb, sargun, keescook, jasowang, devel, linux-fsdevel On Fri, Apr 16, 2021 at 05:58:15PM +0200, Christian Brauner wrote: > They could probably refactor this but I'm not sure why they'd bother. If > they fail processing any of those files they end up aborting the > whole transaction. > (And the original code didn't check the error code btw.) Wait a sec... What does aborting the transaction do to descriptor table? <rereads> Oh, lovely... binder_apply_fd_fixups() is deeply misguided. What it should do is * go through t->fd_fixups, reserving descriptor numbers and putting them into t->buffer (and I'd probably duplicate them into struct binder_txn_fd_fixup). Cleanup in case of failure: go through the list, releasing the descriptors we'd already reserved, doing fput() on fixup->file in all entries and freeing the entries as we go. * On success, go through the list, doing fd_install() and freeing the entries. That's it. No rereading from the buffer, no binder_deferred_fd_close() crap, etc. Again, YOU CAN NOT UNDO fd_install(). Ever. Kernel can not decide it shouldn't have put something in descriptor table and take it back. You can't unring that bell. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] binder: Use receive_fd() to receive file from another process 2021-04-16 17:30 ` Al Viro @ 2021-04-17 1:30 ` Al Viro 0 siblings, 0 replies; 29+ messages in thread From: Al Viro @ 2021-04-17 1:30 UTC (permalink / raw) To: Christian Brauner Cc: Greg KH, Xie Yongji, hch, arve, tkjos, maco, joel, hridya, surenb, sargun, keescook, jasowang, devel, linux-fsdevel On Fri, Apr 16, 2021 at 05:30:41PM +0000, Al Viro wrote: > On Fri, Apr 16, 2021 at 05:58:15PM +0200, Christian Brauner wrote: > > > They could probably refactor this but I'm not sure why they'd bother. If > > they fail processing any of those files they end up aborting the > > whole transaction. > > (And the original code didn't check the error code btw.) > > Wait a sec... What does aborting the transaction do to descriptor table? > <rereads> > Oh, lovely... > > binder_apply_fd_fixups() is deeply misguided. What it should do is > * go through t->fd_fixups, reserving descriptor numbers and > putting them into t->buffer (and I'd probably duplicate them into > struct binder_txn_fd_fixup). Cleanup in case of failure: go through > the list, releasing the descriptors we'd already reserved, doing > fput() on fixup->file in all entries and freeing the entries as > we go. > * On success, go through the list, doing fd_install() and > freeing the entries. Something like this: diff --git a/drivers/android/binder.c b/drivers/android/binder.c index c119736ca56a..b0c5f7e625f3 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2195,6 +2195,7 @@ static int binder_translate_fd(u32 fd, binder_size_t fd_offset, fixup->offset = fd_offset; trace_binder_transaction_fd_send(t, fd, fixup->offset); list_add_tail(&fixup->fixup_entry, &t->fd_fixups); + fixup->target_fd = -1; return ret; @@ -3707,25 +3708,10 @@ static int binder_wait_for_work(struct binder_thread *thread, return ret; } -/** - * binder_apply_fd_fixups() - finish fd translation - * @proc: binder_proc associated @t->buffer - * @t: binder transaction with list of fd fixups - * - * Now that we are in the context of the transaction target - * process, we can allocate and install fds. Process the - * list of fds to translate and fixup the buffer with the - * new fds. - * - * If we fail to allocate an fd, then free the resources by - * fput'ing files that have not been processed and ksys_close'ing - * any fds that have already been allocated. - */ -static int binder_apply_fd_fixups(struct binder_proc *proc, +static int binder_reserve_fds(struct binder_proc *proc, struct binder_transaction *t) { - struct binder_txn_fd_fixup *fixup, *tmp; - int ret = 0; + struct binder_txn_fd_fixup *fixup; list_for_each_entry(fixup, &t->fd_fixups, fixup_entry) { int fd = get_unused_fd_flags(O_CLOEXEC); @@ -3734,42 +3720,55 @@ static int binder_apply_fd_fixups(struct binder_proc *proc, binder_debug(BINDER_DEBUG_TRANSACTION, "failed fd fixup txn %d fd %d\n", t->debug_id, fd); - ret = -ENOMEM; - break; + return -ENOMEM; } binder_debug(BINDER_DEBUG_TRANSACTION, "fd fixup txn %d fd %d\n", t->debug_id, fd); trace_binder_transaction_fd_recv(t, fd, fixup->offset); - fd_install(fd, fixup->file); - fixup->file = NULL; + fixup->target_fd = fd; if (binder_alloc_copy_to_buffer(&proc->alloc, t->buffer, fixup->offset, &fd, - sizeof(u32))) { - ret = -EINVAL; - break; - } + sizeof(u32))) + return -EINVAL; } - list_for_each_entry_safe(fixup, tmp, &t->fd_fixups, fixup_entry) { - if (fixup->file) { - fput(fixup->file); - } else if (ret) { - u32 fd; - int err; - - err = binder_alloc_copy_from_buffer(&proc->alloc, &fd, - t->buffer, - fixup->offset, - sizeof(fd)); - WARN_ON(err); - if (!err) - binder_deferred_fd_close(fd); + return 0; +} + +/** + * binder_apply_fd_fixups() - finish fd translation + * @proc: binder_proc associated @t->buffer + * @t: binder transaction with list of fd fixups + * + * Now that we are in the context of the transaction target + * process, we can allocate fds. Process the list of fds to + * translate and fixup the buffer with the new fds. + * + * If we fail to allocate an fd, then free the resources by + * releasing fds we'd allocated. Otherwise transfer all files + * from fixups to the descriptors we'd allocated for them. + * + * In either case, finish with freeing the fixups. + */ +static int binder_apply_fd_fixups(struct binder_proc *proc, + struct binder_transaction *t) +{ + struct binder_txn_fd_fixup *fixup; + int err = binder_reserve_fds(proc, t); + + if (unlikely(err)) { + list_for_each_entry(fixup, &t->fd_fixups, fixup_entry) { + if (fixup->target_fd >= 0) + put_unused_fd(fixup->target_fd); + } + } else { + list_for_each_entry(fixup, &t->fd_fixups, fixup_entry) { + fd_install(fixup->target_fd, fixup->file); + fixup->file = NULL; } - list_del(&fixup->fixup_entry); - kfree(fixup); } - - return ret; + binder_free_txn_fixups(t); + return err; } static int binder_thread_read(struct binder_proc *proc, diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h index 6cd79011e35d..16ffc5f748ce 100644 --- a/drivers/android/binder_internal.h +++ b/drivers/android/binder_internal.h @@ -497,6 +497,7 @@ struct binder_txn_fd_fixup { struct list_head fixup_entry; struct file *file; size_t offset; + int target_fd; }; struct binder_transaction { ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 0/2] Export receive_fd() to modules and do some cleanups 2021-04-01 9:09 [PATCH 0/2] Export receive_fd() to modules and do some cleanups Xie Yongji 2021-04-01 9:09 ` [PATCH 1/2] file: Export receive_fd() to modules Xie Yongji 2021-04-01 9:09 ` [PATCH 2/2] binder: Use receive_fd() to receive file from another process Xie Yongji @ 2021-04-01 9:53 ` Greg KH 2021-04-01 10:00 ` Yongji Xie 2021-04-01 10:20 ` Christian Brauner 3 siblings, 1 reply; 29+ messages in thread From: Greg KH @ 2021-04-01 9:53 UTC (permalink / raw) To: Xie Yongji Cc: christian.brauner, hch, arve, tkjos, maco, joel, hridya, surenb, viro, sargun, keescook, jasowang, devel, linux-fsdevel On Thu, Apr 01, 2021 at 05:09:30PM +0800, Xie Yongji wrote: > This series starts from Christian's comments on the series[1]. > We'd like to export receive_fd() which can not only be used by > our module in the series[1] but also allow further cleanups > like patch 2 does. But binder can not be a module, so why do you need this? confused, greg k-h ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Re: [PATCH 0/2] Export receive_fd() to modules and do some cleanups 2021-04-01 9:53 ` [PATCH 0/2] Export receive_fd() to modules and do some cleanups Greg KH @ 2021-04-01 10:00 ` Yongji Xie 0 siblings, 0 replies; 29+ messages in thread From: Yongji Xie @ 2021-04-01 10:00 UTC (permalink / raw) To: Greg KH Cc: Christian Brauner, Christoph Hellwig, arve, tkjos, maco, joel, Hridya Valsaraju, Suren Baghdasaryan, viro, Sargun Dhillon, Kees Cook, Jason Wang, devel, linux-fsdevel On Thu, Apr 1, 2021 at 5:53 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Thu, Apr 01, 2021 at 05:09:30PM +0800, Xie Yongji wrote: > > This series starts from Christian's comments on the series[1]. > > We'd like to export receive_fd() which can not only be used by > > our module in the series[1] but also allow further cleanups > > like patch 2 does. > > But binder can not be a module, so why do you need this? > Oh, right. I miss it. Thanks, Yongji ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 0/2] Export receive_fd() to modules and do some cleanups 2021-04-01 9:09 [PATCH 0/2] Export receive_fd() to modules and do some cleanups Xie Yongji ` (2 preceding siblings ...) 2021-04-01 9:53 ` [PATCH 0/2] Export receive_fd() to modules and do some cleanups Greg KH @ 2021-04-01 10:20 ` Christian Brauner 3 siblings, 0 replies; 29+ messages in thread From: Christian Brauner @ 2021-04-01 10:20 UTC (permalink / raw) To: Xie Yongji Cc: hch, gregkh, arve, tkjos, maco, joel, hridya, surenb, viro, sargun, keescook, jasowang, devel, linux-fsdevel On Thu, Apr 01, 2021 at 05:09:30PM +0800, Xie Yongji wrote: > This series starts from Christian's comments on the series[1]. > We'd like to export receive_fd() which can not only be used by > our module in the series[1] but also allow further cleanups > like patch 2 does. > > Now this series is based on Christoph's patch[2]. > > [1] https://lore.kernel.org/linux-fsdevel/20210331080519.172-1-xieyongji@bytedance.com/ > [2] https://lore.kernel.org/linux-fsdevel/20210325082209.1067987-2-hch@lst.de > > Xie Yongji (2): > file: Export receive_fd() to modules > binder: Use receive_fd() to receive file from another process > > drivers/android/binder.c | 4 ++-- > fs/file.c | 6 ++++++ > include/linux/file.h | 7 +++---- Hm, I think we're still talking a bit past each other. I'll try to illustrate what I mean in a patch series soon. Christian ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2021-04-17 1:30 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-01 9:09 [PATCH 0/2] Export receive_fd() to modules and do some cleanups Xie Yongji 2021-04-01 9:09 ` [PATCH 1/2] file: Export receive_fd() to modules Xie Yongji 2021-04-01 9:52 ` Greg KH 2021-04-01 10:24 ` Yongji Xie 2021-04-01 9:09 ` [PATCH 2/2] binder: Use receive_fd() to receive file from another process Xie Yongji 2021-04-01 9:54 ` Greg KH 2021-04-01 10:12 ` Yongji Xie 2021-04-01 10:42 ` Greg KH 2021-04-01 11:29 ` Yongji Xie 2021-04-01 11:33 ` Greg KH 2021-04-01 12:28 ` Yongji Xie 2021-04-01 14:09 ` Greg KH 2021-04-02 9:12 ` Kees Cook 2021-04-01 10:40 ` Christian Brauner 2021-04-01 11:11 ` Yongji Xie 2021-04-16 5:19 ` Al Viro 2021-04-16 5:55 ` Al Viro 2021-04-16 13:42 ` Christian Brauner 2021-04-16 14:09 ` Al Viro 2021-04-16 15:13 ` Christian Brauner 2021-04-16 15:35 ` Al Viro 2021-04-16 15:58 ` Christian Brauner 2021-04-16 16:00 ` Christian Brauner 2021-04-16 17:00 ` Al Viro 2021-04-16 17:30 ` Al Viro 2021-04-17 1:30 ` Al Viro 2021-04-01 9:53 ` [PATCH 0/2] Export receive_fd() to modules and do some cleanups Greg KH 2021-04-01 10:00 ` Yongji Xie 2021-04-01 10:20 ` Christian Brauner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).