kernel crash from regset get() size mismatches

[Roland McGrath]

> To what extent is the regset stuff supposed to tolerate such
> mismatched data?

It ain't.  We don't burden the arch code with the overheads and the
exacting robustness demands of checking for bogus parameters.  (This is the
clear right choice for the arch layer, but that is separate from the issue
of what (thin) safety/convenience layers one might want above that.)

The information required to check that a given pos/count fits the alignment
and size requirements of the arch code is in the struct user_regset fields.
The expectation is that any kernel code driven from userland, where these
parameter values could be unreliable, would validate the parameters against
these constraints.

My expectation is that kernel code calling user_regset hooks can fall into
these categories wrt its pos/count parameters:

* fixed at compile-time: expected to be correct as compiled, just like any
  other buffer overrun, fatal misalignment, etc.

* dynamic but driven only by the user_regset fields: e.g.,
  just uses pos=0, count=regset->size*regset->n, so always valid a priori.
  (This is what core dumps do.)

* "primed" once from userland/whereever, and used repeatedly,
  e.g., a fancy rule-driven thing might have a setup phase where
  it specifies "fetch this part of the regset when that happens":
  should validate parameters once in the setup phase, and then have
  no extra checking overheads when the rule triggers

* fully dynamic, userland/whereever gives requests with pos/count values:
  robustness requires validation of each such request

It's not quite clear that the latter category will ever really exist;
nothing in that category exists as yet.  It may very well be that anything
giving open-ended facilities to userland will only do a "here's the whole
regset" interface (and so be like the existing core dump case).

I didn't provide anything preemptively for the latter two kinds of uses,
but decided to wait and see what nontrivial uses would arise in practice.
Below is a simple helper that we could add to make explicit checks easy.
I didn't include something like this in <linux/regset.h> in the original
upstream submission since it would have had no callers in the kernel.

IMHO it still remains to be seen whether it is actually worthwhile to add
e.g. get/set wrappers that do this check before every call to the arch hooks.


Thanks,
Roland


diff --git a/include/linux/regset.h b/include/linux/regset.h
index 8abee65..fbdf8f9 100644
--- a/include/linux/regset.h
+++ b/include/linux/regset.h
@@ -203,6 +203,37 @@ struct user_regset_view {
  */
 const struct user_regset_view *task_user_regset_view(struct task_struct *tsk);
 
+/**
+ * user_regset_validate_range - check offsets against a &struct user_regset
+ * @regset:	regset being examined
+ * @pos:	offset into the regset data to access, in bytes
+ * @count:	amount of data to copy, in bytes
+ *
+ * Return -%EINVAL if @pos and @count are not valid offsets to pass
+ * in calls to the @regset->get() and @regset->set() functions, or zero
+ * if they are valid.
+ *
+ * The arch functions in &struct user_regset pointers are not expected to
+ * handle bogus @pos or @count arguments gracefully.  Instead their callers
+ * are required to pass a range that complies with the constraints given in
+ * @regset->size, @regset->n, and @regset->align.  This simple helper
+ * function checks putative @pos and @count parameters for validity.
+ *
+ * For efficiency, call this only once when considering a new @pos and
+ * @count from an unchecked source.  Then you can use those same values
+ * many times, with no check at each @regset->get() or @regset->set() call.
+ */
+static inline int user_regset_validate_range(const struct user_regset *regset,
+					     unsigned int pos,
+					     unsigned int count)
+{
+	if (unlikely(pos > regset->size * regset->n ||
+		     (regset->size * regset->n) - pos < count ||
+		     (pos % regset->align) != 0 ||
+		     (count % regset->size) != 0))
+		return -EINVAL;
+	return 0;
+}
 
 /*
  * These are helpers for writing regset get/set functions in arch code.