On Mon, Jan 23, 2023 at 2:18 PM T.J. Mercier <tjmercier@xxxxxxxxxx> wrote:
>
> Any process can cause a memory charge transfer to occur to any other
> process when transmitting a file descriptor through binder. This should
> only be possible for central allocator processes, so the binder object
> flags are added to the security_binder_transfer_file hook so that LSMs
> can enforce restrictions on charge transfers.
>
> Signed-off-by: T.J. Mercier <tjmercier@xxxxxxxxxx>
> ---
> drivers/android/binder.c | 2 +-
> include/linux/lsm_hook_defs.h | 2 +-
> include/linux/lsm_hooks.h | 5 ++++-
> include/linux/security.h | 6 ++++--
> security/security.c | 4 ++--
> security/selinux/hooks.c | 13 ++++++++++++-
> security/selinux/include/classmap.h | 2 +-
> 7 files changed, 25 insertions(+), 9 deletions(-)
...
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 3c5be76a9199..d4cfca3c9a3b 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -88,6 +88,7 @@
> #include <linux/bpf.h>
> #include <linux/kernfs.h>
> #include <linux/stringhash.h> /* for hashlen_string() */
> +#include <uapi/linux/android/binder.h>
> #include <uapi/linux/mount.h>
> #include <linux/fsnotify.h>
> #include <linux/fanotify.h>
> @@ -2029,7 +2030,8 @@ static int selinux_binder_transfer_binder(const struct cred *from,
>
> static int selinux_binder_transfer_file(const struct cred *from,
> const struct cred *to,
> - struct file *file)
> + struct file *file,
> + u32 binder_object_flags)
> {
> u32 sid = cred_sid(to);
> struct file_security_struct *fsec = selinux_file(file);
> @@ -2038,6 +2040,15 @@ static int selinux_binder_transfer_file(const struct cred *from,
> struct common_audit_data ad;
> int rc;
>
> + if (binder_object_flags & BINDER_FD_FLAG_XFER_CHARGE) {
> + rc = avc_has_perm(&selinux_state,
> + cred_sid(from), sid,
> + SECCLASS_BINDER, BINDER__TRANSFER_CHARGE,
> + NULL);
> + if (rc)
> + return rc;
> + }
> +
> ad.type = LSM_AUDIT_DATA_PATH;
> ad.u.path = file->f_path;
>
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index a3c380775d41..2eef180d10d7 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -172,7 +172,7 @@ const struct security_class_mapping secclass_map[] = {
> { "tun_socket",
> { COMMON_SOCK_PERMS, "attach_queue", NULL } },
> { "binder", { "impersonate", "call", "set_context_mgr", "transfer",
> - NULL } },
> + "transfer_charge", NULL } },
> { "cap_userns",
> { COMMON_CAP_PERMS, NULL } },
> { "cap2_userns",
My first take on reading these changes above is that you've completely
ignored my previous comments about SELinux access controls around
resource management. You've leveraged the existing LSM/SELinux hook
as we discussed previously, that's good, but can you explain what
changes you've made to address my concerns about one-off resource
management controls?
--
paul-moore.com
