On 30/11/2022 19:31, Beata Michalska wrote:
On Fri, Nov 25, 2022 at 12:13:26PM +0000, Zachary Leaf wrote:
On 21/11/2022 12:08, Tudor Cretu wrote:
On 21-11-2022 11:40, Beata Michalska wrote:
On Mon, Nov 21, 2022 at 10:57:10AM +0000, Tudor Cretu wrote:
Hi Beata,
On 18-11-2022 00:05, Beata Michalska wrote:
Make copy_struct_from_user capability-aware by switching the actual copying routine to copy_to_user_with_ptr, one that can preserve capability tags throughout the process.
Just a short question: we're keeping both copy_from_user and copy_from_user_with_ptr because we don't want to preserve capability tags by default and each time this is done should be intentional, right? Is there anything specific to copy_struct_from_user that would make it fine to preserve capability tags by default? I think the alternative would be to have two versions: copy_struct_from_user and copy_struct_from_user_with_ptr so that we don't preserve tags by default.
The 'intentional' part here should be applied when copying to userspace, not the other way round, with the stress point being on not providing valid capabilities to user space when not indented, unless I am missing smth (?)
Alright, I guess because there isn't a copy_struct_to_user as well, then we can just do your change without creating confusion. LGMT, thanks for the clarification!
Right, just so it's clear in my mind - if we do copy_*from*_user without _with_ptr here, then we do not preserve cap tags, that means the kernel would not be able to access or otherwise use that pointer?
Yes, regular memcpy routines are not aware of capability registers nor the alignment requirements, so the capability tags (if any) will be lost on loads from the memory. The '_with_ptr' variants use capability registers and take care of the access alignment so the tags can be carried over through loads and stores. As a result the capabilities copied over with those routines are fully preserved. As long as valid capability has been provided (where it was expected), that capability can be used by the kernel to access given memory area. If the regular variants of memcpy were to be used, any memory access through such capability would result in capability tag fault.
Ah yep, that makes sense - since the tag bit is stored out of band unless you explicitly copy that then it will be lost.
I can't think of a scenario where you'd pass a pointer into to kernel but want that pointer unreadable or otherwise unusable.
Capabilities can be used to protect non-pointer types (there are software-defined bits in the permissions set that can be used for that purpose). though we are not dealing with those, at least not at this point. That said, such capabilities should not turn up where actual pointers are expected, not on purpose that is. What led you to thinking about invalid> capability passed as a pointer? Not sure I caught your thought here.
That's good to know as well. I just meant if you are copying some block of memory containing pointers, is there a scenario where you would ever want to use the non-_with_ptr variant to *intentionally* lose the tag bits.
I think Kevin already answered this under the same patch - there are some special cases where you'd want to do that e.g. in the siginfo_t patches[1] some memory/data might be provided to other processes, so we should avoid copying the tags in those cases:
if (current_pid ? copy_from_user_with_ptr(to, from, sizeof(struct kernel_siginfo)) : copy_from_user(to, from, sizeof(struct kernel_siginfo)))
Thanks, Zach
1: [linux-morello] [PATCH 0/6] Handle siginfo_t user pointer tags
BR B.
The copy_*to*_user is the bit where care is needed.
Thanks, Zach
Thanks, Tudor
BR B.
Thanks, Tudor
Signed-off-by: Beata Michalska beata.michalska@arm.com
include/linux/uaccess.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h index 88b2224e85c3..ec31478634cc 100644 --- a/include/linux/uaccess.h +++ b/include/linux/uaccess.h @@ -419,7 +419,7 @@ copy_struct_from_user(void *dst, size_t ksize, const void __user *src, return ret ?: -E2BIG; } /* Copy the interoperable parts of the struct. */ - if (copy_from_user(dst, src, size)) + if (copy_from_user_with_ptr(dst, src, size)) return -EFAULT; return 0; }
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 mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org