linux-kernel - Can KEYCTL_SESSION_TO_PARENT be dropped entirely? -- was Re: [PATCH v2 1/2] KEYS: use synchronous task work for changing parent credentials

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2494949.1723751188@warthog.procyon.org.uk>
Date: Thu, 15 Aug 2024 20:46:28 +0100
From: David Howells <dhowells@...hat.com>
To: Jann Horn <jannh@...gle.com>, Jeffrey Altman <jaltman@...istor.com>,
    openafs-devel@...nafs.org
Cc: dhowells@...hat.com, Paul Moore <paul@...l-moore.com>,
    James Morris <jmorris@...ei.org>,
    "Serge E. Hallyn" <serge@...lyn.com>,
    John Johansen <john.johansen@...onical.com>,
    Jarkko Sakkinen <jarkko@...nel.org>,
    Mickaël Salaün <mic@...ikod.net>,
    Günther Noack <gnoack@...gle.com>,
    Stephen Smalley <stephen.smalley.work@...il.com>,
    Ondrej Mosnacek <omosnace@...hat.com>,
    Casey Schaufler <casey@...aufler-ca.com>,
    linux-afs@...ts.infradead.org, linux-kernel@...r.kernel.org,
    linux-security-module@...r.kernel.org, apparmor@...ts.ubuntu.com,
    keyrings@...r.kernel.org, selinux@...r.kernel.org
Subject: Can KEYCTL_SESSION_TO_PARENT be dropped entirely? -- was Re: [PATCH v2 1/2] KEYS: use synchronous task work for changing parent credentials

Jann Horn <jannh@...gle.com> wrote:

> Rewrite keyctl_session_to_parent() to run task work on the parent
> synchronously, so that any errors that happen in the task work can be
> plumbed back into the syscall return value in the child.

The main thing I worry about is if there's a way to deadlock the child and the
parent against each other.  vfork() for example.

> +	if (task_work_cancel(parent, &ctx.work)) {
> +		/*
> +		 * We got interrupted and the task work was canceled before it
> +		 * could execute.
> +		 * Use -ERESTARTNOINTR instead of -ERESTARTSYS for
> +		 * compatibility - the manpage does not list -EINTR as a
> +		 * possible error for keyctl().
> +		 */

I think returning EINTR is fine, provided that if we return EINTR, the change
didn't happen.  KEYCTL_SESSION_TO_PARENT is only used by the aklog, dlog and
klog* OpenAFS programs AFAIK, and only if "-setpag" is set as a command line
option.  It also won't be effective if you strace the program.

Maybe the AFS people can say whether it's even worth keeping the functionality
rather than just dropping KEYCTL_SESSION_TO_PARENT?

David