* [PATCH 0/2] Use __scm_install_fd() more widely @ 2020-06-10 4:52 Kees Cook 2020-06-10 4:52 ` [PATCH 1/2] net/scm: Regularize compat handling of scm_detach_fds() Kees Cook ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Kees Cook @ 2020-06-10 4:52 UTC (permalink / raw) To: David S. Miller Cc: Kees Cook, Christoph Hellwig, Christian Brauner, Sargun Dhillon, Jakub Kicinski, netdev, linux-kernel Hi, This extends the recent work hch did for scm_detach_fds(), and updates the compat path as well, fixing bugs in the process. Additionally, an effectively incomplete and open-coded __scm_install_fd() is fixed in pidfd_getfd(). Thanks! -Kees Kees Cook (2): net/scm: Regularize compat handling of scm_detach_fds() pidfd: Replace open-coded partial __scm_install_fd() include/net/scm.h | 1 + kernel/pid.c | 12 ++--------- net/compat.c | 55 +++++++++++++++++++++-------------------------- net/core/scm.c | 43 +++++++++++++++++++++++------------- 4 files changed, 56 insertions(+), 55 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] net/scm: Regularize compat handling of scm_detach_fds() 2020-06-10 4:52 [PATCH 0/2] Use __scm_install_fd() more widely Kees Cook @ 2020-06-10 4:52 ` Kees Cook 2020-06-10 4:52 ` [PATCH 2/2] pidfd: Replace open-coded partial __scm_install_fd() Kees Cook 2020-06-10 9:47 ` [PATCH 0/2] Use __scm_install_fd() more widely Christian Brauner 2 siblings, 0 replies; 9+ messages in thread From: Kees Cook @ 2020-06-10 4:52 UTC (permalink / raw) To: David S. Miller Cc: Kees Cook, Christoph Hellwig, Christian Brauner, Sargun Dhillon, Jakub Kicinski, netdev, linux-kernel Duplicate the cleanups from commit 2618d530dd8b ("net/scm: cleanup scm_detach_fds") into the compat code. Also moves the check added in commit 1f466e1f15cf ("net: cleanly handle kernel vs user buffers for ->msg_control") to before the compat call, even though it should be impossible for an in-kernel call to also be compat. Regularized any remaining differences, including a whitespace issue, a checkpatch warning, and adding the check from commit 6900317f5eff ("net, scm: fix PaX detected msg_controllen overflow in scm_detach_fds") which fixed an overflow unique to 64-bit. To avoid confusion when comparing the compat handler to the native handler, just include the same check in the compat handler. Fixes: 48a87cc26c13 ("net: netprio: fd passed in SCM_RIGHTS datagram not set correctly") Fixes: d84295067fc7 ("net: net_cls: fd passed in SCM_RIGHTS datagram not set correctly") Signed-off-by: Kees Cook <keescook@chromium.org> --- AFAICT, the following patches are needed for back-porting this to stable: 0462b6bdb644 ("net: add a CMSG_USER_DATA macro") 2618d530dd8b ("net/scm: cleanup scm_detach_fds") 1f466e1f15cf ("net: cleanly handle kernel vs user buffers for ->msg_control") 6e8a4f9dda38 ("net: ignore sock_from_file errors in __scm_install_fd") --- include/net/scm.h | 1 + net/compat.c | 55 +++++++++++++++++++++-------------------------- net/core/scm.c | 16 +++++++------- 3 files changed, 34 insertions(+), 38 deletions(-) diff --git a/include/net/scm.h b/include/net/scm.h index 1ce365f4c256..4dcc766f15b3 100644 --- a/include/net/scm.h +++ b/include/net/scm.h @@ -37,6 +37,7 @@ struct scm_cookie { #endif }; +int __scm_install_fd(struct file *file, int __user *ufd, int o_flags); void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm); void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm); int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm); diff --git a/net/compat.c b/net/compat.c index 5e3041a2c37d..117f1869bf3b 100644 --- a/net/compat.c +++ b/net/compat.c @@ -281,39 +281,31 @@ int put_cmsg_compat(struct msghdr *kmsg, int level, int type, int len, void *dat return 0; } -void scm_detach_fds_compat(struct msghdr *kmsg, struct scm_cookie *scm) +static int scm_max_fds_compat(struct msghdr *msg) { - struct compat_cmsghdr __user *cm = (struct compat_cmsghdr __user *) kmsg->msg_control; - int fdmax = (kmsg->msg_controllen - sizeof(struct compat_cmsghdr)) / sizeof(int); - int fdnum = scm->fp->count; - struct file **fp = scm->fp->fp; - int __user *cmfptr; - int err = 0, i; + if (msg->msg_controllen <= sizeof(struct compat_cmsghdr)) + return 0; + return (msg->msg_controllen - sizeof(struct compat_cmsghdr)) / sizeof(int); +} - if (fdnum < fdmax) - fdmax = fdnum; +void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm) +{ + struct compat_cmsghdr __user *cm = + (struct compat_cmsghdr __user *)msg->msg_control; + int o_flags = (msg->msg_flags & MSG_CMSG_CLOEXEC) ? O_CLOEXEC : 0; + int fdmax = min_t(int, scm_max_fds_compat(msg), scm->fp->count); + int __user *cmsg_data = CMSG_USER_DATA(cm); + int err = 0, i; - for (i = 0, cmfptr = (int __user *) CMSG_COMPAT_DATA(cm); i < fdmax; i++, cmfptr++) { - int new_fd; - err = security_file_receive(fp[i]); + for (i = 0; i < fdmax; i++) { + err = __scm_install_fd(scm->fp->fp[i], cmsg_data + i, o_flags); if (err) break; - err = get_unused_fd_flags(MSG_CMSG_CLOEXEC & kmsg->msg_flags - ? O_CLOEXEC : 0); - if (err < 0) - break; - new_fd = err; - err = put_user(new_fd, cmfptr); - if (err) { - put_unused_fd(new_fd); - break; - } - /* Bump the usage count and install the file. */ - fd_install(new_fd, get_file(fp[i])); } if (i > 0) { int cmlen = CMSG_COMPAT_LEN(i * sizeof(int)); + err = put_user(SOL_SOCKET, &cm->cmsg_level); if (!err) err = put_user(SCM_RIGHTS, &cm->cmsg_type); @@ -321,16 +313,19 @@ void scm_detach_fds_compat(struct msghdr *kmsg, struct scm_cookie *scm) err = put_user(cmlen, &cm->cmsg_len); if (!err) { cmlen = CMSG_COMPAT_SPACE(i * sizeof(int)); - kmsg->msg_control += cmlen; - kmsg->msg_controllen -= cmlen; + if (msg->msg_controllen < cmlen) + cmlen = msg->msg_controllen; + msg->msg_control += cmlen; + msg->msg_controllen -= cmlen; } } - if (i < fdnum) - kmsg->msg_flags |= MSG_CTRUNC; + + if (i < scm->fp->count || (scm->fp->count && fdmax <= 0)) + msg->msg_flags |= MSG_CTRUNC; /* - * All of the files that fit in the message have had their - * usage counts incremented, so we just free the list. + * All of the files that fit in the message have had their usage counts + * incremented, so we just free the list. */ __scm_destroy(scm); } diff --git a/net/core/scm.c b/net/core/scm.c index 875df1c2989d..86d96152646f 100644 --- a/net/core/scm.c +++ b/net/core/scm.c @@ -280,7 +280,7 @@ void put_cmsg_scm_timestamping(struct msghdr *msg, struct scm_timestamping_inter } EXPORT_SYMBOL(put_cmsg_scm_timestamping); -static int __scm_install_fd(struct file *file, int __user *ufd, int o_flags) +int __scm_install_fd(struct file *file, int __user *ufd, int o_flags) { struct socket *sock; int new_fd; @@ -319,29 +319,29 @@ static int scm_max_fds(struct msghdr *msg) void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm) { - struct cmsghdr __user *cm - = (__force struct cmsghdr __user*)msg->msg_control; + struct cmsghdr __user *cm = + (__force struct cmsghdr __user*)msg->msg_control; int o_flags = (msg->msg_flags & MSG_CMSG_CLOEXEC) ? O_CLOEXEC : 0; int fdmax = min_t(int, scm_max_fds(msg), scm->fp->count); int __user *cmsg_data = CMSG_USER_DATA(cm); int err = 0, i; + /* no use for FD passing from kernel space callers */ + if (WARN_ON_ONCE(!msg->msg_control_is_user)) + return; + if (msg->msg_flags & MSG_CMSG_COMPAT) { scm_detach_fds_compat(msg, scm); return; } - /* no use for FD passing from kernel space callers */ - if (WARN_ON_ONCE(!msg->msg_control_is_user)) - return; - for (i = 0; i < fdmax; i++) { err = __scm_install_fd(scm->fp->fp[i], cmsg_data + i, o_flags); if (err) break; } - if (i > 0) { + if (i > 0) { int cmlen = CMSG_LEN(i * sizeof(int)); err = put_user(SOL_SOCKET, &cm->cmsg_level); -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] pidfd: Replace open-coded partial __scm_install_fd() 2020-06-10 4:52 [PATCH 0/2] Use __scm_install_fd() more widely Kees Cook 2020-06-10 4:52 ` [PATCH 1/2] net/scm: Regularize compat handling of scm_detach_fds() Kees Cook @ 2020-06-10 4:52 ` Kees Cook 2020-06-10 16:45 ` Sargun Dhillon 2020-06-10 9:47 ` [PATCH 0/2] Use __scm_install_fd() more widely Christian Brauner 2 siblings, 1 reply; 9+ messages in thread From: Kees Cook @ 2020-06-10 4:52 UTC (permalink / raw) To: David S. Miller Cc: Kees Cook, Christoph Hellwig, Christian Brauner, Sargun Dhillon, Jakub Kicinski, netdev, linux-kernel The sock counting (sock_update_netprioidx() and sock_update_classid()) was missing from this implementation of fd installation, compared to SCM_RIGHTS. Use the new scm helper to get the work done, after adjusting it to return the installed fd and accept a NULL user pointer. Fixes: 8649c322f75c ("pid: Implement pidfd_getfd syscall") Signed-off-by: Kees Cook <keescook@chromium.org> --- AFAICT, the following patches are needed for back-porting this to stable: 0462b6bdb644 ("net: add a CMSG_USER_DATA macro") 2618d530dd8b ("net/scm: cleanup scm_detach_fds") 1f466e1f15cf ("net: cleanly handle kernel vs user buffers for ->msg_control") 6e8a4f9dda38 ("net: ignore sock_from_file errors in __scm_install_fd") --- kernel/pid.c | 12 ++---------- net/compat.c | 2 +- net/core/scm.c | 27 ++++++++++++++++++++------- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/kernel/pid.c b/kernel/pid.c index f1496b757162..a7ce4ba898d3 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -42,6 +42,7 @@ #include <linux/sched/signal.h> #include <linux/sched/task.h> #include <linux/idr.h> +#include <net/scm.h> struct pid init_struct_pid = { .count = REFCOUNT_INIT(1), @@ -635,18 +636,9 @@ static int pidfd_getfd(struct pid *pid, int fd) if (IS_ERR(file)) return PTR_ERR(file); - ret = security_file_receive(file); - if (ret) { - fput(file); - return ret; - } - - ret = get_unused_fd_flags(O_CLOEXEC); + ret = __scm_install_fd(file, NULL, O_CLOEXEC); if (ret < 0) fput(file); - else - fd_install(ret, file); - return ret; } diff --git a/net/compat.c b/net/compat.c index 117f1869bf3b..f8575555b6d7 100644 --- a/net/compat.c +++ b/net/compat.c @@ -299,7 +299,7 @@ void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm) for (i = 0; i < fdmax; i++) { err = __scm_install_fd(scm->fp->fp[i], cmsg_data + i, o_flags); - if (err) + if (err < 0) break; } diff --git a/net/core/scm.c b/net/core/scm.c index 86d96152646f..e80648fb4da7 100644 --- a/net/core/scm.c +++ b/net/core/scm.c @@ -280,6 +280,14 @@ void put_cmsg_scm_timestamping(struct msghdr *msg, struct scm_timestamping_inter } EXPORT_SYMBOL(put_cmsg_scm_timestamping); +/** + * __scm_install_fd() - Install received file into file descriptor table + * + * Installs a received file into the file descriptor table, with appropriate + * checks and count updates. + * + * Returns fd installed or -ve on error. + */ int __scm_install_fd(struct file *file, int __user *ufd, int o_flags) { struct socket *sock; @@ -294,20 +302,25 @@ int __scm_install_fd(struct file *file, int __user *ufd, int o_flags) if (new_fd < 0) return new_fd; - error = put_user(new_fd, ufd); - if (error) { - put_unused_fd(new_fd); - return error; + if (ufd) { + error = put_user(new_fd, ufd); + if (error) { + put_unused_fd(new_fd); + return error; + } } - /* Bump the usage count and install the file. */ + /* Bump the usage count and install the file. The resulting value of + * "error" is ignored here since we only need to take action when + * the file is a socket and testing "sock" for NULL is sufficient. + */ sock = sock_from_file(file, &error); if (sock) { sock_update_netprioidx(&sock->sk->sk_cgrp_data); sock_update_classid(&sock->sk->sk_cgrp_data); } fd_install(new_fd, get_file(file)); - return 0; + return new_fd; } static int scm_max_fds(struct msghdr *msg) @@ -337,7 +350,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm) for (i = 0; i < fdmax; i++) { err = __scm_install_fd(scm->fp->fp[i], cmsg_data + i, o_flags); - if (err) + if (err < 0) break; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] pidfd: Replace open-coded partial __scm_install_fd() 2020-06-10 4:52 ` [PATCH 2/2] pidfd: Replace open-coded partial __scm_install_fd() Kees Cook @ 2020-06-10 16:45 ` Sargun Dhillon 0 siblings, 0 replies; 9+ messages in thread From: Sargun Dhillon @ 2020-06-10 16:45 UTC (permalink / raw) To: Kees Cook Cc: David S. Miller, Christoph Hellwig, Christian Brauner, Jakub Kicinski, netdev, linux-kernel On Tue, Jun 09, 2020 at 09:52:14PM -0700, Kees Cook wrote: > The sock counting (sock_update_netprioidx() and sock_update_classid()) > was missing from this implementation of fd installation, compared to > SCM_RIGHTS. Use the new scm helper to get the work done, after adjusting > it to return the installed fd and accept a NULL user pointer. > > Fixes: 8649c322f75c ("pid: Implement pidfd_getfd syscall") > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > AFAICT, the following patches are needed for back-porting this to stable: > > 0462b6bdb644 ("net: add a CMSG_USER_DATA macro") > 2618d530dd8b ("net/scm: cleanup scm_detach_fds") > 1f466e1f15cf ("net: cleanly handle kernel vs user buffers for ->msg_control") > 6e8a4f9dda38 ("net: ignore sock_from_file errors in __scm_install_fd") > --- > kernel/pid.c | 12 ++---------- > net/compat.c | 2 +- > net/core/scm.c | 27 ++++++++++++++++++++------- > 3 files changed, 23 insertions(+), 18 deletions(-) > > diff --git a/kernel/pid.c b/kernel/pid.c > index f1496b757162..a7ce4ba898d3 100644 > --- a/kernel/pid.c > +++ b/kernel/pid.c > @@ -42,6 +42,7 @@ > #include <linux/sched/signal.h> > #include <linux/sched/task.h> > #include <linux/idr.h> > +#include <net/scm.h> > > struct pid init_struct_pid = { > .count = REFCOUNT_INIT(1), > @@ -635,18 +636,9 @@ static int pidfd_getfd(struct pid *pid, int fd) > if (IS_ERR(file)) > return PTR_ERR(file); > > - ret = security_file_receive(file); > - if (ret) { > - fput(file); > - return ret; > - } > - > - ret = get_unused_fd_flags(O_CLOEXEC); > + ret = __scm_install_fd(file, NULL, O_CLOEXEC); > if (ret < 0) > fput(file); > - else > - fd_install(ret, file); > - > return ret; > } > > diff --git a/net/compat.c b/net/compat.c > index 117f1869bf3b..f8575555b6d7 100644 > --- a/net/compat.c > +++ b/net/compat.c > @@ -299,7 +299,7 @@ void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm) > > for (i = 0; i < fdmax; i++) { > err = __scm_install_fd(scm->fp->fp[i], cmsg_data + i, o_flags); > - if (err) > + if (err < 0) > break; > } > > diff --git a/net/core/scm.c b/net/core/scm.c > index 86d96152646f..e80648fb4da7 100644 > --- a/net/core/scm.c > +++ b/net/core/scm.c > @@ -280,6 +280,14 @@ void put_cmsg_scm_timestamping(struct msghdr *msg, struct scm_timestamping_inter > } > EXPORT_SYMBOL(put_cmsg_scm_timestamping); > > +/** > + * __scm_install_fd() - Install received file into file descriptor table Any reason not to rename this remote_install_* or similar, and move it to fs/? > + * > + * Installs a received file into the file descriptor table, with appropriate > + * checks and count updates. > + * > + * Returns fd installed or -ve on error. > + */ > int __scm_install_fd(struct file *file, int __user *ufd, int o_flags) > { > struct socket *sock; > @@ -294,20 +302,25 @@ int __scm_install_fd(struct file *file, int __user *ufd, int o_flags) > if (new_fd < 0) > return new_fd; > > - error = put_user(new_fd, ufd); > - if (error) { > - put_unused_fd(new_fd); > - return error; > + if (ufd) { See my comment elsewhere about not being able to use NULL here. > + error = put_user(new_fd, ufd); > + if (error) { > + put_unused_fd(new_fd); > + return error; > + } > } > > - /* Bump the usage count and install the file. */ > + /* Bump the usage count and install the file. The resulting value of > + * "error" is ignored here since we only need to take action when > + * the file is a socket and testing "sock" for NULL is sufficient. > + */ > sock = sock_from_file(file, &error); > if (sock) { > sock_update_netprioidx(&sock->sk->sk_cgrp_data); > sock_update_classid(&sock->sk->sk_cgrp_data); > } > fd_install(new_fd, get_file(file)); > - return 0; > + return new_fd; > } > > static int scm_max_fds(struct msghdr *msg) > @@ -337,7 +350,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm) > > for (i = 0; i < fdmax; i++) { > err = __scm_install_fd(scm->fp->fp[i], cmsg_data + i, o_flags); > - if (err) > + if (err < 0) > break; > } > > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Use __scm_install_fd() more widely 2020-06-10 4:52 [PATCH 0/2] Use __scm_install_fd() more widely Kees Cook 2020-06-10 4:52 ` [PATCH 1/2] net/scm: Regularize compat handling of scm_detach_fds() Kees Cook 2020-06-10 4:52 ` [PATCH 2/2] pidfd: Replace open-coded partial __scm_install_fd() Kees Cook @ 2020-06-10 9:47 ` Christian Brauner 2020-06-10 14:52 ` Kees Cook 2020-06-10 17:03 ` Kees Cook 2 siblings, 2 replies; 9+ messages in thread From: Christian Brauner @ 2020-06-10 9:47 UTC (permalink / raw) To: Kees Cook Cc: David S. Miller, Christoph Hellwig, Christian Brauner, Sargun Dhillon, Jakub Kicinski, netdev, linux-kernel On Tue, Jun 09, 2020 at 09:52:12PM -0700, Kees Cook wrote: > Hi, > > This extends the recent work hch did for scm_detach_fds(), and updates > the compat path as well, fixing bugs in the process. Additionally, > an effectively incomplete and open-coded __scm_install_fd() is fixed > in pidfd_getfd(). Since __scm_detach_fds() becomes something that is available outside of net/* should we provide a static inline wrapper under a different name? The "socket-level control message" prefix seems a bit odd in pidfd_getfd() and - once we make use of it there - seccomp. I'd suggest we do: static inline int fd_install_received(struct file *file, unsigned int flags) { return __scm_install_fd(file, NULL, flags); } which can be called in pidfd_getfd() and once we have other callers that want the additional put_user() (e.g. seccomp_ in there we simply add: static inline fd_install_user(struct file *file, unsigned int flags, int __user *ufd) { return __scm_install_fd(file, ufd, flags); } and seems the wrappers both could happily live in the fs part of the world? Christian ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Use __scm_install_fd() more widely 2020-06-10 9:47 ` [PATCH 0/2] Use __scm_install_fd() more widely Christian Brauner @ 2020-06-10 14:52 ` Kees Cook 2020-06-10 17:03 ` Kees Cook 1 sibling, 0 replies; 9+ messages in thread From: Kees Cook @ 2020-06-10 14:52 UTC (permalink / raw) To: Christian Brauner Cc: David S. Miller, Christoph Hellwig, Christian Brauner, Sargun Dhillon, Jakub Kicinski, netdev, linux-kernel On Wed, Jun 10, 2020 at 11:47:35AM +0200, Christian Brauner wrote: > On Tue, Jun 09, 2020 at 09:52:12PM -0700, Kees Cook wrote: > > Hi, > > > > This extends the recent work hch did for scm_detach_fds(), and updates > > the compat path as well, fixing bugs in the process. Additionally, > > an effectively incomplete and open-coded __scm_install_fd() is fixed > > in pidfd_getfd(). > > Since __scm_detach_fds() becomes something that is available outside of > net/* should we provide a static inline wrapper under a different name? The > "socket-level control message" prefix seems a bit odd in pidfd_getfd() > and - once we make use of it there - seccomp. > > I'd suggest we do: > > static inline int fd_install_received(struct file *file, unsigned int flags) > { > return __scm_install_fd(file, NULL, flags); > } > > which can be called in pidfd_getfd() and once we have other callers that > want the additional put_user() (e.g. seccomp_ in there we simply add: > > static inline fd_install_user(struct file *file, unsigned int flags, int __user *ufd) > { > return __scm_install_fd(file, ufd, flags); > } > > and seems the wrappers both could happily live in the fs part of the world? Yeah, this seems good. I also note that randconfigs are kicking back my series as broken when CONFIG_NET=n (oops), so this needs some refactoring before patch 2. -- Kees Cook ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Use __scm_install_fd() more widely 2020-06-10 9:47 ` [PATCH 0/2] Use __scm_install_fd() more widely Christian Brauner 2020-06-10 14:52 ` Kees Cook @ 2020-06-10 17:03 ` Kees Cook 2020-06-10 18:38 ` Jakub Kicinski 1 sibling, 1 reply; 9+ messages in thread From: Kees Cook @ 2020-06-10 17:03 UTC (permalink / raw) To: Christian Brauner Cc: David S. Miller, Christoph Hellwig, Christian Brauner, Sargun Dhillon, Jakub Kicinski, netdev, linux-kernel On Wed, Jun 10, 2020 at 11:47:35AM +0200, Christian Brauner wrote: > On Tue, Jun 09, 2020 at 09:52:12PM -0700, Kees Cook wrote: > > Hi, > > > > This extends the recent work hch did for scm_detach_fds(), and updates > > the compat path as well, fixing bugs in the process. Additionally, > > an effectively incomplete and open-coded __scm_install_fd() is fixed > > in pidfd_getfd(). > > Since __scm_detach_fds() becomes something that is available outside of > net/* should we provide a static inline wrapper under a different name? The > "socket-level control message" prefix seems a bit odd in pidfd_getfd() > and - once we make use of it there - seccomp. > > I'd suggest we do: > > static inline int fd_install_received(struct file *file, unsigned int flags) > { > return __scm_install_fd(file, NULL, flags); > } > > which can be called in pidfd_getfd() and once we have other callers that > want the additional put_user() (e.g. seccomp_ in there we simply add: > > static inline fd_install_user(struct file *file, unsigned int flags, int __user *ufd) > { > return __scm_install_fd(file, ufd, flags); > } > > and seems the wrappers both could happily live in the fs part of the world? I combined your and Sargun's suggestions. (It can't live in any more net/core/scm.c in the case of CONFIG_NET=n, but the wrappers make the changes much nicer looking.) https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=devel/seccomp/addfd/v3.2 If 0-day doesn't kick anything back on this tree, I'll resend the series... -- Kees Cook ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Use __scm_install_fd() more widely 2020-06-10 17:03 ` Kees Cook @ 2020-06-10 18:38 ` Jakub Kicinski 2020-06-10 19:45 ` Kees Cook 0 siblings, 1 reply; 9+ messages in thread From: Jakub Kicinski @ 2020-06-10 18:38 UTC (permalink / raw) To: Kees Cook Cc: Christian Brauner, David S. Miller, Christoph Hellwig, Christian Brauner, Sargun Dhillon, netdev, linux-kernel On Wed, 10 Jun 2020 10:03:03 -0700 Kees Cook wrote: > If 0-day doesn't kick anything back on this tree, I'll resend the > series... Well, 0-day may find more, but I can already tell you that patch 1 has a checkpatch error: ERROR: "(foo*)" should be "(foo *)" #149: FILE: net/core/scm.c:323: + (__force struct cmsghdr __user*)msg->msg_control; total: 1 errors, 0 warnings, 0 checks, 131 lines checked And patch 2 makes W=1 builds unhappy: net/core/scm.c:292: warning: Function parameter or member 'file' not described in '__scm_install_fd' net/core/scm.c:292: warning: Function parameter or member 'ufd' not described in '__scm_install_fd' net/core/scm.c:292: warning: Function parameter or member 'o_flags' not described in '__scm_install_fd' :) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Use __scm_install_fd() more widely 2020-06-10 18:38 ` Jakub Kicinski @ 2020-06-10 19:45 ` Kees Cook 0 siblings, 0 replies; 9+ messages in thread From: Kees Cook @ 2020-06-10 19:45 UTC (permalink / raw) To: Jakub Kicinski Cc: Christian Brauner, David S. Miller, Christoph Hellwig, Christian Brauner, Sargun Dhillon, netdev, linux-kernel On Wed, Jun 10, 2020 at 11:38:00AM -0700, Jakub Kicinski wrote: > On Wed, 10 Jun 2020 10:03:03 -0700 Kees Cook wrote: > > If 0-day doesn't kick anything back on this tree, I'll resend the > > series... > > Well, 0-day may find more, but I can already tell you that patch 1 has > a checkpatch error: > > ERROR: "(foo*)" should be "(foo *)" > #149: FILE: net/core/scm.c:323: > + (__force struct cmsghdr __user*)msg->msg_control; > > total: 1 errors, 0 warnings, 0 checks, 131 lines checked > > And patch 2 makes W=1 builds unhappy: > > net/core/scm.c:292: warning: Function parameter or member 'file' not described in '__scm_install_fd' > net/core/scm.c:292: warning: Function parameter or member 'ufd' not described in '__scm_install_fd' > net/core/scm.c:292: warning: Function parameter or member 'o_flags' not described in '__scm_install_fd' Yeah, there are a few more problems too. I'll get it nailed down hopefully this week and send a v2. Thanks for looking at it! -- Kees Cook ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-06-10 19:45 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-10 4:52 [PATCH 0/2] Use __scm_install_fd() more widely Kees Cook 2020-06-10 4:52 ` [PATCH 1/2] net/scm: Regularize compat handling of scm_detach_fds() Kees Cook 2020-06-10 4:52 ` [PATCH 2/2] pidfd: Replace open-coded partial __scm_install_fd() Kees Cook 2020-06-10 16:45 ` Sargun Dhillon 2020-06-10 9:47 ` [PATCH 0/2] Use __scm_install_fd() more widely Christian Brauner 2020-06-10 14:52 ` Kees Cook 2020-06-10 17:03 ` Kees Cook 2020-06-10 18:38 ` Jakub Kicinski 2020-06-10 19:45 ` Kees Cook
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).