usermode-helper code oddity query..

[Linus Torvalds]

So this is a bit out of the blue, but I cleaned up some really old
legacy capability code in commit f122a08b197d ("capability: just use a
'u64' instead of a 'u32[2]' array") and in the process I became the
last person to touch kernel/umh.c.

Tag, I'm clearly it. Not that I want to take that glory away from
PeterZ, who was the previous last person to touch that code. In fact,
I'm just cc'ing everybody who has been touching that file at all in
the last years, and a few /proc sysctl maintainers too.

Anyway, I wanted to try to keep the capability code cleanups clear,
and really only touched the data structure conversion, but I'm just
left staring at that code and wondering why we have those odd CAP_BSET
/ CAP_PI dummy pointers. They've been there since the whole /proc
interface was introduced, but they seem strangely pointless.

It would _seem_ like it would be a lot simpler and more
straightforward to just put the actual pointer to the usermodehelper
capability in there instead, and not have that odd fake pointer
enumeration at all.

IOW, I'm wondering what's wrong with a patch like the attached. I
might be missing something.

I also would have like that array to be an array of "u32" rather than
"unsigned long" (because that is, sadly, the interface we have, like
it or not), but we don't seem to have a proc_dou32vec_minmax(). I
guess "uint" is the same thing, but it's not pretty. Anyway, that's a
separate and independent issue from this.

And no, none of this is important. Just random cleanup of code I
happened to look at for other reasons.

           Linus
 kernel/umh.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/kernel/umh.c b/kernel/umh.c
index 2a4708277335..60aa9e764a38 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -32,9 +32,6 @@
 
 #include <trace/events/module.h>
 
-#define CAP_BSET	(void *)1
-#define CAP_PI		(void *)2
-
 static kernel_cap_t usermodehelper_bset = CAP_FULL_SET;
 static kernel_cap_t usermodehelper_inheritable = CAP_FULL_SET;
 static DEFINE_SPINLOCK(umh_sysctl_lock);
@@ -512,16 +509,11 @@ static int proc_cap_handler(struct ctl_table *table, int write,
 	/*
 	 * convert from the global kernel_cap_t to the ulong array to print to
 	 * userspace if this is a read.
+	 *
+	 * Legacy format: capabilities are exposed as two 32-bit values
 	 */
+	cap = table->data;
 	spin_lock(&umh_sysctl_lock);
-	if (table->data == CAP_BSET)
-		cap = &usermodehelper_bset;
-	else if (table->data == CAP_PI)
-		cap = &usermodehelper_inheritable;
-	else
-		BUG();
-
-	/* Legacy format: capabilities are exposed as two 32-bit values */
 	cap_array[0] = (u32) cap->val;
 	cap_array[1] = cap->val >> 32;
 	spin_unlock(&umh_sysctl_lock);
@@ -555,14 +547,14 @@ static int proc_cap_handler(struct ctl_table *table, int write,
 struct ctl_table usermodehelper_table[] = {
 	{
 		.procname	= "bset",
-		.data		= CAP_BSET,
+		.data		= &usermodehelper_bset,
 		.maxlen		= 2 * sizeof(unsigned long),
 		.mode		= 0600,
 		.proc_handler	= proc_cap_handler,
 	},
 	{
 		.procname	= "inheritable",
-		.data		= CAP_PI,
+		.data		= &usermodehelper_inheritable,
 		.maxlen		= 2 * sizeof(unsigned long),
 		.mode		= 0600,
 		.proc_handler	= proc_cap_handler,