On Thu, Sep 23 2021 at 14:26, Greg KH wrote:

On Mon, Sep 13, 2021 at 01:01:25PM -0700, Sohil Mehta wrote:

quoted

+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