Re: Oops (request_key_auth_describe) while running cve-2016-7042 from LTP

On Fri, 30 Aug 2019 12:18:07 +0530 Sachin Sant wrote:

[ 8074.351033] BUG: Kernel NULL pointer dereference at 0x00000038
[ 8074.351046] Faulting instruction address: 0xc0000000004ddf30
[ 8074.351052] Oops: Kernel access of bad area, sig: 11 [#1]
[ 8074.351056] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries

Add rcu gp.

--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c

@@ -64,12 +64,19 @@ static int request_key_auth_instantiate(
 static void request_key_auth_describe(const struct key *key,
 				      struct seq_file *m)
 {
-	struct request_key_auth *rka = dereference_key_rcu(key);
+	struct request_key_auth *rka;
+
+	rcu_read_lock();
+	rka = dereference_key_rcu(key);
+	if (!rka)
+		goto out;
 
 	seq_puts(m, "key:");
 	seq_puts(m, key->description);
 	if (key_is_positive(key))
 		seq_printf(m, " pid:%d ci:%zu", rka->pid, rka->callout_len);
+out:
+	rcu_read_unlock();
 }
 
 /*

Thanks for the patch. Works for me. Test ran fine without any problems.

Tested-by: Sachin Sant <redacted>

Thanks
-Sachin

Hillf Danton [off-list ref] wrote:

-	struct request_key_auth *rka = dereference_key_rcu(key);
+	struct request_key_auth *rka;
+
+	rcu_read_lock();
+	rka = dereference_key_rcu(key);

This shouldn't help as the caller, proc_keys_show(), is holding the RCU read
lock across the call.  The end of the function reads:

		if (key->type->describe)
			key->type->describe(key, m);
		seq_putc(m, '\n');

		rcu_read_unlock();
		return 0;
	}

and the documentation says "This method will be called with the RCU read lock
held".

I suspect the actual bugfix is this bit:

+	if (!rka)
+		goto out;

David

David Howells [off-list ref] wrote:

Hillf Danton [off-list ref] wrote:

quoted

-	struct request_key_auth *rka = dereference_key_rcu(key);
+	struct request_key_auth *rka;
+
+	rcu_read_lock();
+	rka = dereference_key_rcu(key);

This shouldn't help as the caller, proc_keys_show(), is holding the RCU read
lock across the call.  The end of the function reads:
and the documentation says "This method will be called with the RCU read lock
held".

1, callee has no pre defined duty to help caller in general; they should
not try to do anything, however, to help their callers in principle due to
limited info on their hands IMO.
2, uses of rcu can be nested.
3, no comment can be found in security/keys/request_key_auth.c about
the rcu already documented.
4, the newly added rcu can avoid incidental messup anywhere else.

Hillf