On Tue, Jul 07, 2020 at 02:22:20PM +0200, Christian Brauner wrote:
> On Mon, Jul 06, 2020 at 01:17:17PM -0700, Kees Cook wrote:
> > The sock counting (sock_update_netprioidx() and sock_update_classid()) was
> > missing from pidfd's implementation of received fd installation. Replace
> > the open-coded version with a call to the new receive_fd()
> > helper.
> >
> > Thanks to Vamshi K Sthambamkadi <vamshi.k.sthambamkadi@xxxxxxxxx> for
> > catching a missed fput() in an earlier version of this patch.
> >
> > Fixes: 8649c322f75c ("pid: Implement pidfd_getfd syscall")
> > Reviewed-by: Sargun Dhillon <sargun@xxxxxxxxx>
> > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> > ---
>
> Thanks!
> Acked-by: Christian Brauner <christian.brauner@xxxxxxxxxx>
>
> Christoph, Kees,
>
> So while the patch is correct it leaves 5.6 and 5.7 with a bug in the
> pidfd_getfd() implementation and that just doesn't seem right. I'm
> wondering whether we should introduce:
>
> void sock_update(struct file *file)
> {
> struct socket *sock;
> int error;
>
> sock = sock_from_file(file, &error);
> if (sock) {
> sock_update_netprioidx(&sock->sk->sk_cgrp_data);
> sock_update_classid(&sock->sk->sk_cgrp_data);
> }
> }
>
> and switch pidfd_getfd() over to:
>
> diff --git a/kernel/pid.c b/kernel/pid.c
> index f1496b757162..c26bba822be3 100644
> --- a/kernel/pid.c
> +++ b/kernel/pid.c
> @@ -642,10 +642,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 {
> + sock_update(file);
> fd_install(ret, file);
> + }
>
> return ret;
> }
>
> first thing in the series and then all of the other patches on top of it
> so that we can Cc stable for this and that can get it backported to 5.6,
> 5.7, and 5.8.
>
> Alternatively, I can make this a separate bugfix patch series which I'll
> send upstream soonish. Or we have specific patches just for 5.6, 5.7,
> and 5.8. Thoughts?
I was thinking of just tossing the entire series (hch's and mine) at
-stable since it's relatively narrow. I'll look at what's needed for
backports...
>
> Thanks!
> Christian
--
Kees Cook
