On Wed, Jun 17, 2020 at 03:03:24PM -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 fd_install_received() helper.
Fixes: 8649c322f75c ("pid: Implement pidfd_getfd syscall") Signed-off-by: Kees Cook keescook@chromium.org
kernel/pid.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/kernel/pid.c b/kernel/pid.c index f1496b757162..24924ec5df0e 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -635,18 +635,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 = fd_install_received(file, O_CLOEXEC); if (ret < 0) fput(file);
- else
fd_install(ret, file);
So someone just sent a fix for pidfd_getfd() that was based on the changes done here.
I've been on vacation so didn't have a change to review this series and I see it's already in linux-next. This introduces a memory leak and actually proves a point I tried to stress when adding this helper: fd_install_received() in contrast to fd_install() does _not_ consume a reference because it takes one before it calls into fd_install(). That means, you need an unconditional fput() here both in the failure and error path. I strongly suggest though that we simply align the behavior between fd_install() and fd_install_received() and have the latter simply consume a reference when it succeeds! Imho, this bug proves that I was right to insist on this before. ;)
Thanks! Christian