On Wed, May 25, 2022 at 2:37 PM Frederick Lawler [off-list ref] wrote:

While experimenting with the security_prepare_creds() LSM hook, we
noticed that our EPERM error code was not propagated up the callstack.
Instead ENOMEM is always returned.  As a result, some tools may send a
confusing error message to the user:

$ unshare -rU
unshare: unshare failed: Cannot allocate memory

A user would think that the system didn't have enough memory, when
instead the action was denied.

This problem occurs because prepare_creds() and prepare_kernel_cred()
return NULL when security_prepare_creds() returns an error code. Later,
functions calling prepare_creds() and prepare_kernel_cred() return
ENOMEM because they assume that a NULL meant there was no memory
allocated.

Fix this by propagating an error code from security_prepare_creds() up
the callstack.

Signed-off-by: Frederick Lawler <redacted>

---
Changes since v1:
- Revert style churn in ovl_create_or_link() noted by Amir
- Revert style churn in prepare_nsset() noted by Serge
- Update documentation for prepare_creds()
- Set ofs->creator_cred in ovl_fill_super() and req->creds in aio_fsync()
  to NULL on error noted by Amir
---
 Documentation/security/credentials.rst |  6 +++---
 fs/aio.c                               |  9 +++++++--
 fs/cachefiles/security.c               |  8 ++++----
 fs/cifs/cifs_spnego.c                  |  4 ++--
 fs/cifs/cifsacl.c                      |  4 ++--
 fs/coredump.c                          |  2 +-
 fs/exec.c                              | 14 ++++++++-----
 fs/ksmbd/smb_common.c                  |  4 ++--
 fs/nfs/flexfilelayout/flexfilelayout.c |  7 +++++--
 fs/nfs/nfs4idmap.c                     |  4 ++--
 fs/nfsd/auth.c                         |  4 ++--
 fs/nfsd/nfs4callback.c                 | 10 ++++-----
 fs/nfsd/nfs4recover.c                  |  4 ++--
 fs/nfsd/nfsfh.c                        |  4 ++--
 fs/open.c                              |  8 ++++----
 fs/overlayfs/dir.c                     |  6 ++++--
 fs/overlayfs/super.c                   |  6 ++++--
 kernel/capability.c                    |  4 ++--
 kernel/cred.c                          | 28 +++++++++++++++-----------
 kernel/groups.c                        |  4 ++--
 kernel/nsproxy.c                       |  9 ++++++++-
 kernel/sys.c                           | 28 +++++++++++++-------------
 kernel/trace/trace_events_user.c       |  4 ++--
 kernel/umh.c                           |  5 +++--
 kernel/user_namespace.c                |  6 ++++--
 net/dns_resolver/dns_key.c             |  4 ++--
 security/apparmor/task.c               | 12 +++++------
 security/commoncap.c                   | 20 +++++++++---------
 security/keys/keyctl.c                 |  8 ++++----
 security/keys/process_keys.c           | 16 +++++++--------
 security/landlock/syscalls.c           |  4 ++--
 security/selinux/hooks.c               |  8 ++++----
 security/smack/smack_lsm.c             |  8 ++++----
 security/smack/smackfs.c               |  4 ++--
 34 files changed, 153 insertions(+), 123 deletions(-)

The SELinux bits look fine to me.

Acked-by: Paul Moore <paul@paul-moore.com> (SELinux)

-- 
paul-moore.com