On Sat, 5 Oct 2024 at 17:06, Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
>
> On Fri, Oct 4, 2024 at 2:48 PM Luca Boccassi <luca.boccassi@xxxxxxxxx> wrote:
> > On Wed, 2 Oct 2024 at 15:48, Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> > > On Wed, Oct 2, 2024 at 10:25 AM <luca.boccassi@xxxxxxxxx> wrote:
>
> ...
>
> > > [NOTE: please CC the LSM list on changes like this]
> > >
> > > Thanks Luca :)
> > >
> > > With the addition of the LSM syscalls we've created a lsm_ctx struct
> > > (see include/uapi/linux/lsm.h) that properly supports multiple LSMs.
> > > The original char ptr "secctx" approach worked back when only a single
> > > LSM was supported at any given time, but now that multiple LSMs are
> > > supported we need something richer, and it would be good to use this
> > > new struct in any new userspace API.
> > >
> > > See the lsm_get_self_attr(2) syscall for an example (defined in
> > > security/lsm_syscalls.c but effectively implemented via
> > > security_getselfattr() in security/security.c).
> >
> > Thanks for the review, makes sense to me - I had a look at those
> > examples but unfortunately it is getting a bit beyond my (very low)
> > kernel skills, so I've dropped the string-based security_context from
> > v2 but without adding something else, is there someone more familiar
> > with the LSM world that could help implementing that side?
>
> We are running a little short on devs/time in LSM land right now so I
> guess I'm the only real option (not that I have any time, but you know
> how it goes). If you can put together the ioctl side of things I can
> likely put together the LSM side fairly quickly - sound good?
Here's a skeleton ioctl, needs lsm-specific fields to be added to the new struct, and filled in the new function:
diff --git a/fs/pidfs.c b/fs/pidfs.c
index 4eaf30873105..30310489f5e1 100644
--- a/fs/pidfs.c
+++ b/fs/pidfs.c
@@ -115,6 +115,35 @@ static __poll_t pidfd_poll(struct file *file, struct poll_table_struct *pts)
return poll_flags;
}
+static long pidfd_security(struct task_struct *task, unsigned int cmd, unsigned long arg)
+{
+ struct pidfd_security __user *usecurity = (struct pidfd_security __user *)arg;
+ size_t usize = _IOC_SIZE(cmd);
+ struct pidfd_security ksecurity = {};
+ __u64 mask;
+
+ if (!usecurity)
+ return -EINVAL;
+ if (usize < PIDFD_SECURITY_SIZE_VER0)
+ return -EINVAL; /* First version, no smaller struct possible */
+
+ if (copy_from_user(&mask, &usecurity->mask, sizeof(mask)))
+ return -EFAULT;
+
+ // TODO: fill in ksecurity
+
+ /*
+ * If userspace and the kernel have the same struct size it can just
+ * be copied. If userspace provides an older struct, only the bits that
+ * userspace knows about will be copied. If userspace provides a new
+ * struct, only the bits that the kernel knows about will be copied.
+ */
+ if (copy_to_user(usecurity, &ksecurity, min(usize, sizeof(ksecurity))))
+ return -EFAULT;
+
+ return 0;
+}
+
static long pidfd_info(struct task_struct *task, unsigned int cmd, unsigned long arg)
{
struct pidfd_info __user *uinfo = (struct pidfd_info __user *)arg;
@@ -160,7 +189,7 @@ static long pidfd_info(struct task_struct *task, unsigned int cmd, unsigned long
* the flag only if we can still access it.
*/
rcu_read_lock();
- cgrp = task_dfl_cgroup(current);
+ cgrp = task_dfl_cgroup(task);
if (cgrp) {
kinfo.cgroupid = cgroup_id(cgrp);
kinfo.mask |= PIDFD_INFO_CGROUPID;
@@ -209,9 +238,11 @@ static long pidfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
if (!task)
return -ESRCH;
- /* Extensible IOCTL that does not open namespace FDs, take a shortcut */
+ /* Extensible IOCTLs that do not open namespace FDs, take a shortcut */
if (_IOC_NR(cmd) == _IOC_NR(PIDFD_GET_INFO))
return pidfd_info(task, cmd, arg);
+ if (_IOC_NR(cmd) == _IOC_NR(PIDFD_GET_SECURITY))
+ return pidfd_security(task, cmd, arg);
if (arg)
return -EINVAL;
diff --git a/include/uapi/linux/pidfd.h b/include/uapi/linux/pidfd.h
index 4540f6301b8c..0658880a9089 100644
--- a/include/uapi/linux/pidfd.h
+++ b/include/uapi/linux/pidfd.h
@@ -65,6 +65,14 @@ struct pidfd_info {
__u32 spare0[1];
};
+#define PIDFD_SECURITY_SIZE_VER0 8 /* sizeof first published struct */
+
+struct pidfd_security {
+ /* This mask follows the same rules as pidfd_info.mask. */
+ __u64 mask;
+ // TODO: add data fields and flags and increase size defined above
+};
+
#define PIDFS_IOCTL_MAGIC 0xFF
#define PIDFD_GET_CGROUP_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 1)
@@ -78,5 +86,6 @@ struct pidfd_info {
#define PIDFD_GET_USER_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 9)
#define PIDFD_GET_UTS_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 10)
#define PIDFD_GET_INFO _IOWR(PIDFS_IOCTL_MAGIC, 11, struct pidfd_info)
+#define PIDFD_GET_SECURITY _IOWR(PIDFS_IOCTL_MAGIC, 12, struct pidfd_security)
#endif /* _UAPI_LINUX_PIDFD_H */
