This series fixes up the epoll system calls to make it possible for purecap apps to pass a pointer through epoll_data. This includes changing the UAPI header.
The patches can also be found at [1].
[1] https://git.morello-project.org/kristina/linux/-/commits/epoll_v1
Kristina Martsenko (4): epoll: Relax epitem size check for CHERI systems epoll: Add compat handling of epoll_event epoll: Use user pointer types for epoll_event.data epoll: Add TODO for CHERI security enforcement
fs/eventpoll.c | 29 ++++++++++++++++++++++++----- include/linux/eventpoll.h | 21 +++++++++++++++++++-- include/uapi/linux/eventpoll.h | 2 +- 3 files changed, 44 insertions(+), 8 deletions(-)
base-commit: 6e72e8dd44266dbcece028a018c252a54dd23e3e
For performance reasons eventpoll_init() currently checks that the size of struct epitem is kept under 128 bytes. It avoids making this check for 128-bit and larger systems, as on those systems struct epitem would be above 128 bytes in size.
On a CONFIG_CHERI_PURECAP_UABI kernel on a CHERI system, 'long' and 'void *' remain 64-bit but 'void __user *' is 128-bit which causes struct epitem to go over 128 bytes in size. Relax the check for such systems as well.
Signed-off-by: Kristina Martsenko kristina.martsenko@arm.com --- fs/eventpoll.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c index e2daa940ebce..e5c9a29f0da1 100644 --- a/fs/eventpoll.c +++ b/fs/eventpoll.c @@ -2376,7 +2376,8 @@ static int __init eventpoll_init(void) * We can have many thousands of epitems, so prevent this from * using an extra cache line on 64-bit (and smaller) CPUs */ - BUILD_BUG_ON(sizeof(void *) <= 8 && sizeof(struct epitem) > 128); + if (sizeof(void *) <= 8 && sizeof(void __user *) <= 8) + BUILD_BUG_ON(sizeof(struct epitem) > 128);
/* Allocates slab cache used to allocate "struct epitem" items */ epi_cache = kmem_cache_create("eventpoll_epi", sizeof(struct epitem),
A subsequent patch is going to change struct epoll_event to enable it to support new architectures. On such architectures, the current struct layout still needs to be supported for compat tasks. In preparation for that change, add a struct compat_epoll_event and handle it in epoll syscalls.
Note, it may be cleaner to have a separate compat entry point for epoll_ctl. For now, use only the native entry point to avoid changing the syscall table on all architectures.
Signed-off-by: Kristina Martsenko kristina.martsenko@arm.com --- fs/eventpoll.c | 22 ++++++++++++++++++---- include/linux/eventpoll.h | 17 +++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c index e5c9a29f0da1..8b9e4fdda28e 100644 --- a/fs/eventpoll.c +++ b/fs/eventpoll.c @@ -2185,9 +2185,21 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd, { struct epoll_event epds;
- if (ep_op_has_event(op) && - copy_from_user(&epds, event, sizeof(struct epoll_event))) - return -EFAULT; + if (ep_op_has_event(op)) { + if (in_compat_syscall()) { + struct compat_epoll_event compat_epds; + + if (copy_from_user(&compat_epds, event, + sizeof(struct compat_epoll_event))) + return -EFAULT; + + epds.events = compat_epds.events; + epds.data = compat_epds.data; + } else { + if (copy_from_user(&epds, event, sizeof(struct epoll_event))) + return -EFAULT; + } + }
return do_epoll_ctl(epfd, op, fd, &epds, false); } @@ -2202,13 +2214,15 @@ static int do_epoll_wait(int epfd, struct epoll_event __user *events, int error; struct fd f; struct eventpoll *ep; + size_t event_size = in_compat_syscall() ? sizeof(struct compat_epoll_event) + : sizeof(struct epoll_event);
/* The maximum number of event must be greater than zero */ if (maxevents <= 0 || maxevents > EP_MAX_EVENTS) return -EINVAL;
/* Verify that the area passed by the user is writeable */ - if (!access_ok(events, maxevents * sizeof(struct epoll_event))) + if (!access_ok(events, maxevents * event_size)) return -EFAULT;
/* Get the "struct file *" for the eventpoll file */ diff --git a/include/linux/eventpoll.h b/include/linux/eventpoll.h index 3337745d81bd..15acfb6771c3 100644 --- a/include/linux/eventpoll.h +++ b/include/linux/eventpoll.h @@ -8,6 +8,7 @@ #ifndef _LINUX_EVENTPOLL_H #define _LINUX_EVENTPOLL_H
+#include <linux/compat.h> #include <uapi/linux/eventpoll.h> #include <uapi/linux/kcmp.h>
@@ -68,6 +69,11 @@ static inline void eventpoll_release(struct file *file) {}
#endif
+struct compat_epoll_event { + __poll_t events; + __u64 data; +}; + #if defined(CONFIG_ARM) && defined(CONFIG_OABI_COMPAT) /* ARM OABI has an incompatible struct layout and needs a special handler */ extern struct epoll_event __user * @@ -78,6 +84,17 @@ static inline struct epoll_event __user * epoll_put_uevent(__poll_t revents, __u64 data, struct epoll_event __user *uevent) { + if (in_compat_syscall()) { + struct compat_epoll_event __user *compat_uevent = + (struct compat_epoll_event __user *)uevent; + + if (__put_user(revents, &compat_uevent->events) || + __put_user(data, &compat_uevent->data)) + return NULL; + + return (struct epoll_event __user *)(compat_uevent+1); + } + if (__put_user(revents, &uevent->events) || __put_user(data, &uevent->data)) return NULL;
The 'data' member of struct epoll_event may contain a user pointer. On some architectures (for example CHERI) a user pointer is a 129-bit capability, so the __u64 type is not big enough to hold it. Use the __kernel_uintptr_t type instead, which is big enough on the affected architectures while remaining 64-bit on others.
In addition, special copy routines need to be used when copying user pointers to and from userspace. Use these for epoll_event.data.
Signed-off-by: Kristina Martsenko kristina.martsenko@arm.com --- fs/eventpoll.c | 5 +++-- include/linux/eventpoll.h | 6 +++--- include/uapi/linux/eventpoll.h | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c index 8b9e4fdda28e..8a1d3b3fe9c9 100644 --- a/fs/eventpoll.c +++ b/fs/eventpoll.c @@ -2194,9 +2194,10 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd, return -EFAULT;
epds.events = compat_epds.events; - epds.data = compat_epds.data; + epds.data = (__kernel_uintptr_t)as_user_ptr(compat_epds.data); } else { - if (copy_from_user(&epds, event, sizeof(struct epoll_event))) + if (copy_from_user_with_ptr(&epds, event, + sizeof(struct epoll_event))) return -EFAULT; } } diff --git a/include/linux/eventpoll.h b/include/linux/eventpoll.h index 15acfb6771c3..457811d82ff2 100644 --- a/include/linux/eventpoll.h +++ b/include/linux/eventpoll.h @@ -81,7 +81,7 @@ epoll_put_uevent(__poll_t revents, __u64 data, struct epoll_event __user *uevent); #else static inline struct epoll_event __user * -epoll_put_uevent(__poll_t revents, __u64 data, +epoll_put_uevent(__poll_t revents, __kernel_uintptr_t data, struct epoll_event __user *uevent) { if (in_compat_syscall()) { @@ -89,14 +89,14 @@ epoll_put_uevent(__poll_t revents, __u64 data, (struct compat_epoll_event __user *)uevent;
if (__put_user(revents, &compat_uevent->events) || - __put_user(data, &compat_uevent->data)) + __put_user((__u64)data, &compat_uevent->data)) return NULL;
return (struct epoll_event __user *)(compat_uevent+1); }
if (__put_user(revents, &uevent->events) || - __put_user(data, &uevent->data)) + __put_user_ptr(data, &uevent->data)) return NULL;
return uevent+1; diff --git a/include/uapi/linux/eventpoll.h b/include/uapi/linux/eventpoll.h index 8a3432d0f0dc..ee17d8995caa 100644 --- a/include/uapi/linux/eventpoll.h +++ b/include/uapi/linux/eventpoll.h @@ -76,7 +76,7 @@
struct epoll_event { __poll_t events; - __u64 data; + __kernel_uintptr_t data; } EPOLL_PACKED;
#ifdef CONFIG_PM_SLEEP
An epoll fd may be inherited during fork and execve, allowing processes to currently pass capabilities to each other via epoll_event->data. However, the CHERI security model does not allow sharing capabilities via any form of IPC [1]. Therefore we should restrict when capabilities can be passed.
Add a TODO to revisit this in the future.
It's worth noting that kqueue/epoll-shim in CheriBSD likely suffers from the same problem.
[1] https://git.morello-project.org/morello/kernel/linux/-/wikis/Morello-pure-ca...
Signed-off-by: Kristina Martsenko kristina.martsenko@arm.com --- fs/eventpoll.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c index 8a1d3b3fe9c9..5dce236cf3ec 100644 --- a/fs/eventpoll.c +++ b/fs/eventpoll.c @@ -2179,6 +2179,9 @@ int do_epoll_ctl(int epfd, int op, int fd, struct epoll_event *epds, * The following function implements the controller interface for * the eventpoll file that enables the insertion/removal/change of * file descriptors inside the interest set. + * + * TODO [PCuABI] - restrict passing capabilities between processes via + * epoll_ctl/epoll_wait */ SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd, struct epoll_event __user *, event)
On 16/08/2022 18:32, Kristina Martsenko wrote:
This series fixes up the epoll system calls to make it possible for purecap apps to pass a pointer through epoll_data. This includes changing the UAPI header.
The patches can also be found at [1].
[1] https://git.morello-project.org/kristina/linux/-/commits/epoll_v1
Kristina Martsenko (4): epoll: Relax epitem size check for CHERI systems epoll: Add compat handling of epoll_event epoll: Use user pointer types for epoll_event.data epoll: Add TODO for CHERI security enforcement
Happy with this series, will wait a bit for further opinions before merging.
Kevin
fs/eventpoll.c | 29 ++++++++++++++++++++++++----- include/linux/eventpoll.h | 21 +++++++++++++++++++-- include/uapi/linux/eventpoll.h | 2 +- 3 files changed, 44 insertions(+), 8 deletions(-)
base-commit: 6e72e8dd44266dbcece028a018c252a54dd23e3e
On 8/16/22 22:02, Kristina Martsenko wrote:
This series fixes up the epoll system calls to make it possible for purecap apps to pass a pointer through epoll_data. This includes changing the UAPI header.
The patches can also be found at [1].
[1] https://git.morello-project.org/kristina/linux/-/commits/epoll_v1
The epoll changes for purecap uABI look fine to me.
Thanks, Amit Daniel
Kristina Martsenko (4): epoll: Relax epitem size check for CHERI systems epoll: Add compat handling of epoll_event epoll: Use user pointer types for epoll_event.data epoll: Add TODO for CHERI security enforcement
fs/eventpoll.c | 29 ++++++++++++++++++++++++----- include/linux/eventpoll.h | 21 +++++++++++++++++++-- include/uapi/linux/eventpoll.h | 2 +- 3 files changed, 44 insertions(+), 8 deletions(-)
base-commit: 6e72e8dd44266dbcece028a018c252a54dd23e3e
On 25/08/2022 14:01, Amit Kachhap wrote:
On 8/16/22 22:02, Kristina Martsenko wrote:
This series fixes up the epoll system calls to make it possible for purecap apps to pass a pointer through epoll_data. This includes changing the UAPI header.
The patches can also be found at [1].
[1] https://git.morello-project.org/kristina/linux/-/commits/epoll_v1
The epoll changes for purecap uABI look fine to me.
Thanks for confirming! Now applied on next, thanks Kristina!
Kevin
Thanks, Amit Daniel
Kristina Martsenko (4): epoll: Relax epitem size check for CHERI systems epoll: Add compat handling of epoll_event epoll: Use user pointer types for epoll_event.data epoll: Add TODO for CHERI security enforcement
fs/eventpoll.c | 29 ++++++++++++++++++++++++----- include/linux/eventpoll.h | 21 +++++++++++++++++++-- include/uapi/linux/eventpoll.h | 2 +- 3 files changed, 44 insertions(+), 8 deletions(-)
base-commit: 6e72e8dd44266dbcece028a018c252a54dd23e3e
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
linux-morello@op-lists.linaro.org
-
Amit Kachhap -
Kevin Brodsky -
Kristina Martsenko