On Thu, Sep 23 2021 at 14:26, Greg KH wrote:
On Mon, Sep 13, 2021 at 01:01:25PM -0700, Sohil Mehta wrote:
+SYSCALL_DEFINE2(uintr_register_handler, u64 __user *, handler, unsigned int, flags) +{
- int ret;
- if (!uintr_arch_enabled())
return -EOPNOTSUPP;- if (flags)
return -EINVAL;- /* TODO: Validate the handler address */
- if (!handler)
return -EFAULT;Um, that's a pretty big "TODO" here.
How are you going to define what is, and what is not, an allowed "handler"?
The requirement is obviously that this is a valid user space address, but that's so hard to validate that it needs to be done later.
At least the documentation claims that a non user space address should result in a #GP on delivery. Whether that holds in all corner cases (see the spurious handling muck) is a different question and might come back to us later through a channel which we hate with a passion :)
I'm sure the security people would like to get involved here, as well as the auditing people. Have you talked with them about their requirements for this type of stuff?
The handler is strictly a user space address and user space is generally allowed to shoot itself into the foot. If the address is bogus then this will resolve into inaccessible, not-mapped or not exectuable space and the application can keep the pieces.
Whether the hardware handles the resulting exception correctly is a different question, but that can't be prevented by any sanity check on the address at registration time.
Thanks,
tglx