[PATCH 11/11] moduleparam: Drop legacy kernel_param_ops .get_str field and dispatch logic

[Kees Cook]

All struct kernel_param_ops .get callbacks have been migrated to using
struct seq_buf. Drop the migration scaffolding.

Signed-off-by: Kees Cook <kees at kernel.org>
---
 include/linux/moduleparam.h | 37 ++--------------------
 kernel/params.c             | 62 ++++++++++---------------------------
 2 files changed, 20 insertions(+), 79 deletions(-)

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 38acb5aef56b..e6af6f051c93 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -66,15 +66,8 @@ struct kernel_param_ops {
 	/*
 	 * Format the parameter's value into @s.  Return 0 on success
 	 * (length derived from seq_buf_used()) or -errno on error.
-	 * Exactly one of .get and .get_str should be set; the dispatcher
-	 * WARNs and prefers .get if both are.
 	 */
 	int (*get)(struct seq_buf *s, const struct kernel_param *kp);
-	/*
-	 * Returns length written or -errno.  Buffer is 4k (ie. be short!).
-	 * Deprecated: callbacks should implement .get instead.
-	 */
-	int (*get_str)(char *buffer, const struct kernel_param *kp);
 	/* Optional function to free kp->arg when module unloaded. */
 	void (*free)(void *arg);
 };
@@ -84,33 +77,11 @@ struct kernel_param_ops {
  * any required visibility qualifiers (typically "static"):
  *
  *   static DEFINE_KERNEL_PARAM_OPS(my_ops, my_set, my_get);
- *
- * @_get may be either of:
- *   int (*)(struct seq_buf *, const struct kernel_param *) (seq_buf)
- *   int (*)(char *, const struct kernel_param *)           (legacy)
- *
- * The macro uses _Generic to route the function pointer to the
- * matching field (.get or .get_str) at compile time, leaving the
- * other field NULL. Each helper matches the wrong prototype signature
- * and returns NULL, falling through to the default branch otherwise;
- * if @_get has neither expected signature the assignment to the
- * fields gets a normal compile-time type-mismatch error.
  */
-#define _KERNEL_PARAM_OPS_GET(_get)					\
-	_Generic((_get),						\
-	    int (*)(char *, const struct kernel_param *): NULL,		\
-	    default: (_get))
-
-#define _KERNEL_PARAM_OPS_GET_STR(_get)					\
-	_Generic((_get),						\
-	    int (*)(struct seq_buf *, const struct kernel_param *): NULL, \
-	    default: (_get))
-
 #define DEFINE_KERNEL_PARAM_OPS(_name, _set, _get)			\
 	const struct kernel_param_ops _name = {				\
 		.set = (_set),						\
-		.get = _KERNEL_PARAM_OPS_GET(_get),			\
-		.get_str = _KERNEL_PARAM_OPS_GET_STR(_get),		\
+		.get = (_get),						\
 	}
 
 /* As DEFINE_KERNEL_PARAM_OPS, with KERNEL_PARAM_OPS_FL_NOARG set. */
@@ -118,16 +89,14 @@ struct kernel_param_ops {
 	const struct kernel_param_ops _name = {				\
 		.flags = KERNEL_PARAM_OPS_FL_NOARG,			\
 		.set = (_set),						\
-		.get = _KERNEL_PARAM_OPS_GET(_get),			\
-		.get_str = _KERNEL_PARAM_OPS_GET_STR(_get),		\
+		.get = (_get),						\
 	}
 
 /* As DEFINE_KERNEL_PARAM_OPS, with an additional .free callback. */
 #define DEFINE_KERNEL_PARAM_OPS_FREE(_name, _set, _get, _free)		\
 	const struct kernel_param_ops _name = {				\
 		.set = (_set),						\
-		.get = _KERNEL_PARAM_OPS_GET(_get),			\
-		.get_str = _KERNEL_PARAM_OPS_GET_STR(_get),		\
+		.get = (_get),						\
 		.free = (_free),					\
 	}
 
diff --git a/kernel/params.c b/kernel/params.c
index 25f0c8d5d19f..6b410189297b 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -461,8 +461,7 @@ static int param_array_get(struct seq_buf *s, const struct kernel_param *kp)
 {
 	const struct kparam_array *arr = kp->arr;
 	struct kernel_param p = *kp;
-	char *elem_buf = NULL;
-	int i, ret = 0;
+	int i, ret;
 
 	for (i = 0; i < (arr->num ? *arr->num : arr->max); i++) {
 		size_t before = s->len;
@@ -470,23 +469,9 @@ static int param_array_get(struct seq_buf *s, const struct kernel_param *kp)
 		p.arg = arr->elem + arr->elemsize * i;
 		check_kparam_locked(p.mod);
 
-		if (arr->ops->get) {
-			ret = arr->ops->get(s, &p);
-			if (ret < 0)
-				goto out;
-		} else {
-			if (!elem_buf) {
-				elem_buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
-				if (!elem_buf) {
-					ret = -ENOMEM;
-					goto out;
-				}
-			}
-			ret = arr->ops->get_str(elem_buf, &p);
-			if (ret < 0)
-				goto out;
-			seq_buf_putmem(s, elem_buf, ret);
-		}
+		ret = arr->ops->get(s, &p);
+		if (ret < 0)
+			return ret;
 
 		/* Nothing got written (e.g. overflow) — stop. */
 		if (s->len == before)
@@ -496,10 +481,7 @@ static int param_array_get(struct seq_buf *s, const struct kernel_param *kp)
 		if (i && s->buffer[before - 1] == '\n')
 			s->buffer[before - 1] = ',';
 	}
-	ret = 0;
-out:
-	kfree(elem_buf);
-	return ret;
+	return 0;
 }
 
 static void param_array_free(void *arg)
@@ -570,32 +552,22 @@ static ssize_t param_attr_show(const struct module_attribute *mattr,
 	int count;
 	const struct param_attribute *attribute = to_param_attr(mattr);
 	const struct kernel_param_ops *ops = attribute->param->ops;
+	struct seq_buf s;
 
-	if (!ops->get && !ops->get_str)
+	if (!ops->get)
 		return -EPERM;
 
-	WARN_ON_ONCE(ops->get && ops->get_str);
-
 	kernel_param_lock(mk->mod);
-	if (ops->get) {
-		struct seq_buf s;
-
-		seq_buf_init(&s, buf, PAGE_SIZE);
-		count = ops->get(&s, attribute->param);
-		if (count >= 0) {
-			WARN_ON_ONCE(count > 0);
-			count = seq_buf_used(&s);
-			/* Make sure string is terminated. */
-			seq_buf_str(&s);
-			/*
-			 * If overflowed, reduce count by 1 for trailing
-			 * NUL byte.
-			 */
-			if (seq_buf_has_overflowed(&s))
-				count--;
-		}
-	} else {
-		count = ops->get_str(buf, attribute->param);
+	seq_buf_init(&s, buf, PAGE_SIZE);
+	count = ops->get(&s, attribute->param);
+	if (count >= 0) {
+		WARN_ON_ONCE(count > 0);
+		count = seq_buf_used(&s);
+		/* Make sure string is terminated. */
+		seq_buf_str(&s);
+		/* If overflowed, reduce count by 1 for trailing NUL byte. */
+		if (seq_buf_has_overflowed(&s))
+			count--;
 	}
 	kernel_param_unlock(mk->mod);
 	return count;
-- 
2.34.1