| Message ID | 20200709182642.1773477-3-keescook@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add seccomp notifier ioctl that enables adding fds | expand |
On Thu, Jul 9, 2020 at 8:26 PM Kees Cook <keescook@chromium.org> wrote: > The sock counting (sock_update_netprioidx() and sock_update_classid()) > was missing from pidfd's implementation of received fd installation. Add > a call to the new __receive_sock() helper. [...] > diff --git a/kernel/pid.c b/kernel/pid.c [...] > @@ -642,10 +643,12 @@ static int pidfd_getfd(struct pid *pid, int fd) > } > > ret = get_unused_fd_flags(O_CLOEXEC); > - if (ret < 0) > + if (ret < 0) { > fput(file); > - else > + } else { > fd_install(ret, file); > + __receive_sock(file); > + } __receive_sock() has to be before fd_install(), otherwise `file` can be a dangling pointer.
On Thu, Jul 09, 2020 at 10:00:42PM +0200, Jann Horn wrote: > On Thu, Jul 9, 2020 at 8:26 PM Kees Cook <keescook@chromium.org> wrote: > > The sock counting (sock_update_netprioidx() and sock_update_classid()) > > was missing from pidfd's implementation of received fd installation. Add > > a call to the new __receive_sock() helper. > [...] > > diff --git a/kernel/pid.c b/kernel/pid.c > [...] > > @@ -642,10 +643,12 @@ static int pidfd_getfd(struct pid *pid, int fd) > > } > > > > ret = get_unused_fd_flags(O_CLOEXEC); > > - if (ret < 0) > > + if (ret < 0) { > > fput(file); > > - else > > + } else { > > fd_install(ret, file); > > + __receive_sock(file); > > + } > > __receive_sock() has to be before fd_install(), otherwise `file` can > be a dangling pointer. Burned by fd_install()'s API again. Thanks. I will respin.
On Thu, Jul 09, 2020 at 10:00:42PM +0200, Jann Horn wrote: > On Thu, Jul 9, 2020 at 8:26 PM Kees Cook <keescook@chromium.org> wrote: > > The sock counting (sock_update_netprioidx() and sock_update_classid()) > > was missing from pidfd's implementation of received fd installation. Add > > a call to the new __receive_sock() helper. > [...] > > diff --git a/kernel/pid.c b/kernel/pid.c > [...] > > @@ -642,10 +643,12 @@ static int pidfd_getfd(struct pid *pid, int fd) > > } > > > > ret = get_unused_fd_flags(O_CLOEXEC); > > - if (ret < 0) > > + if (ret < 0) { > > fput(file); > > - else > > + } else { > > fd_install(ret, file); > > + __receive_sock(file); > > + } > > __receive_sock() has to be before fd_install(), otherwise `file` can > be a dangling pointer. I've swapped the order now and double-checked the other uses. Everything else seems fine.
diff --git a/kernel/pid.c b/kernel/pid.c index f1496b757162..85ed00abdc7c 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/sock.h> struct pid init_struct_pid = { .count = REFCOUNT_INIT(1), @@ -642,10 +643,12 @@ static int pidfd_getfd(struct pid *pid, int fd) } ret = get_unused_fd_flags(O_CLOEXEC); - if (ret < 0) + if (ret < 0) { fput(file); - else + } else { fd_install(ret, file); + __receive_sock(file); + } return ret; }
The sock counting (sock_update_netprioidx() and sock_update_classid()) was missing from pidfd's implementation of received fd installation. Add a call to the new __receive_sock() helper. Cc: stable@vger.kernel.org Fixes: 8649c322f75c ("pid: Implement pidfd_getfd syscall") Signed-off-by: Kees Cook <keescook@chromium.org> --- kernel/pid.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)