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

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

[Sachin Sant]

> On 30-Aug-2019, at 2:26 PM, Hillf Danton <hdanton@sina.com> wrote:
> 
> 
> 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;
> 

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

Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>

Thanks
-Sachin

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

[David Howells]

Hillf Danton <hdanton@sina.com> 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

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

[David Howells]

Hillf Danton <hdanton@sina.com> wrote:

> 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.

Ah, no.  It's entirely reasonable for an API to specify that one of its
methods will be called with one or more locks held - and that the method must
be aware of this and may make use of this.

> 3, no comment can be found in security/keys/request_key_auth.c about
> the rcu already documented.

There is API documentation in Documentation/security/keys/core.rst.  If you
look at about line 1538 onwards:

  *  ``void (*describe)(const struct key *key, struct seq_file *p);``

     This method is optional. It is called during /proc/keys reading to
     summarise a key's description and payload in text form.

     This method will be called with the RCU read lock held. rcu_dereference()
     should be used to read the payload pointer if the payload is to be
     accessed. key->datalen cannot be trusted to stay consistent with the
     contents of the payload.

     The description will not change, though the key's state may.

     It is not safe to sleep in this method; the RCU read lock is held by the
     caller.

David

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

[David Howells]

Hi Hillf,

Would you like to me to put you down as the author of this patch?  If so, I'll
need a Signed-off-by from you.

David
---
commit df882ad6d4e24a3763719c1798ea58e87d56c2d7
Author: Hillf Danton <hdanton@sina.com>
Date:   Fri Aug 30 15:54:33 2019 +0100

    keys: Fix missing null pointer check in request_key_auth_describe()
    
    If a request_key authentication token key gets revoked, there's a window in
    which request_key_auth_describe() can see it with a NULL payload - but it
    makes no check for this and something like the following oops may occur:
    
            BUG: Kernel NULL pointer dereference at 0x00000038
            Faulting instruction address: 0xc0000000004ddf30
            Oops: Kernel access of bad area, sig: 11 [#1]
            ...
            NIP [...] request_key_auth_describe+0x90/0xd0
            LR [...] request_key_auth_describe+0x54/0xd0
            Call Trace:
            [...] request_key_auth_describe+0x54/0xd0 (unreliable)
            [...] proc_keys_show+0x308/0x4c0
            [...] seq_read+0x3d0/0x540
            [...] proc_reg_read+0x90/0x110
            [...] __vfs_read+0x3c/0x70
            [...] vfs_read+0xb4/0x1b0
            [...] ksys_read+0x7c/0x130
            [...] system_call+0x5c/0x70
    
    Fix this by checking for a NULL pointer when describing such a key.
    
    Also make the read routine check for a NULL pointer to be on the safe side.
    
    Fixes: 04c567d9313e ("[PATCH] Keys: Fix race between two instantiators of a key")
    Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
    Signed-off-by: David Howells <dhowells@redhat.com>
    Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>

diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index e73ec040e250..ecba39c93fd9 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -66,6 +66,9 @@ static void request_key_auth_describe(const struct key *key,
 {
 	struct request_key_auth *rka = dereference_key_rcu(key);
 
+	if (!rka)
+		return;
+
 	seq_puts(m, "key:");
 	seq_puts(m, key->description);
 	if (key_is_positive(key))
@@ -83,6 +86,9 @@ static long request_key_auth_read(const struct key *key,
 	size_t datalen;
 	long ret;
 
+	if (!rka)
+		return -EKEYREVOKED;
+
 	datalen = rka->callout_len;
 	ret = datalen;
 

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

[Sachin Sant]

> On 30-Aug-2019, at 8:43 PM, David Howells <dhowells@redhat.com> wrote:
> 
> Can you try this patch instead of Hillf’s?

Works for me. Test ran fine without any problem.

Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>

Thanks
-Sachin

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

[David Howells]

Can you try this patch instead of Hillf's?

David
---
commit df882ad6d4e24a3763719c1798ea58e87d56c2d7
Author: Hillf Danton <hdanton@sina.com>
Date:   Fri Aug 30 15:54:33 2019 +0100

    keys: Fix missing null pointer check in request_key_auth_describe()
    
    If a request_key authentication token key gets revoked, there's a window in
    which request_key_auth_describe() can see it with a NULL payload - but it
    makes no check for this and something like the following oops may occur:
    
            BUG: Kernel NULL pointer dereference at 0x00000038
            Faulting instruction address: 0xc0000000004ddf30
            Oops: Kernel access of bad area, sig: 11 [#1]
            ...
            NIP [...] request_key_auth_describe+0x90/0xd0
            LR [...] request_key_auth_describe+0x54/0xd0
            Call Trace:
            [...] request_key_auth_describe+0x54/0xd0 (unreliable)
            [...] proc_keys_show+0x308/0x4c0
            [...] seq_read+0x3d0/0x540
            [...] proc_reg_read+0x90/0x110
            [...] __vfs_read+0x3c/0x70
            [...] vfs_read+0xb4/0x1b0
            [...] ksys_read+0x7c/0x130
            [...] system_call+0x5c/0x70
    
    Fix this by checking for a NULL pointer when describing such a key.
    
    Also make the read routine check for a NULL pointer to be on the safe side.
    
    Fixes: 04c567d9313e ("[PATCH] Keys: Fix race between two instantiators of a key")
    Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
    Signed-off-by: David Howells <dhowells@redhat.com>

diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index e73ec040e250..ecba39c93fd9 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -66,6 +66,9 @@ static void request_key_auth_describe(const struct key *key,
 {
 	struct request_key_auth *rka = dereference_key_rcu(key);
 
+	if (!rka)
+		return;
+
 	seq_puts(m, "key:");
 	seq_puts(m, key->description);
 	if (key_is_positive(key))
@@ -83,6 +86,9 @@ static long request_key_auth_read(const struct key *key,
 	size_t datalen;
 	long ret;
 
+	if (!rka)
+		return -EKEYREVOKED;
+
 	datalen = rka->callout_len;
 	ret = datalen;
 

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

[Sachin Sant]

While running LTP tests (specifically cve-2016-7042) against 5.3-rc6
(commit 4a64489cf8) on a POWER9 LPAR, following problem is seen

[ 3373.814425] FS-Cache: Netfs 'nfs' registered for caching
[ 7695.250230] Clock: inserting leap second 23:59:60 UTC
[ 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
[ 8074.351067] Dumping ftrace buffer:
[ 8074.351081]    (ftrace buffer empty)
[ 8074.351085] Modules linked in: nfsv3 nfs_acl nfs lockd grace fscache sctp tun brd vfat fat fuse xfs overlay loop iscsi_target_mod target_core_mod macsec tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag binfmt_misc bridge stp llc ip6t_rpfilter ipt_REJECT nf_reject_ipv4 ip6t_REJECT nf_reject_ipv6 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute ip6table_nat ip6table_mangle ip6table_raw iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c iptable_mangle iptable_raw ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter sunrpc uio_pdrv_genirq pseries_rng sg uio ip_tables ext4 mbcache jbd2 sr_mod cdrom sd_mod ibmvscsi ibmveth scsi_transport_srp dm_mirror dm_region_hash dm_log dm_mod [last unloaded: dummy_del_mod]
[ 8074.351153] CPU: 10 PID: 8314 Comm: cve-2016-7042 Tainted: G           O      5.3.0-rc6-autotest #1
[ 8074.351158] NIP:  c0000000004ddf30 LR: c0000000004ddef4 CTR: c0000000004ddea0
[ 8074.351164] REGS: c0000000e74fb800 TRAP: 0300   Tainted: G           O       (5.3.0-rc6-autotest)
[ 8074.351170] MSR:  8000000000009033 <SF,EE,ME,IR,DR,RI,LE>  CR: 88002482  XER: 00000000
[ 8074.351177] CFAR: c00000000000dfc4 DAR: 0000000000000038 DSISR: 40000000 IRQMASK: 0 
[ 8074.351177] GPR00: c0000000004ddef4 c0000000e74fba90 c0000000013cc200 c0000008b0d7039b 
[ 8074.351177] GPR04: c0000008b0dabe3e 0000000000000007 00090a0200000904 c0000008b0d80000 
[ 8074.351177] GPR08: 00000000000003a2 0000000000000001 000000000000039b c000000000d03ac0 
[ 8074.351177] GPR12: c0000000004ddea0 c00000001ec5dc00 0000000000000000 0000000000000000 
[ 8074.351177] GPR16: 0000000000000000 0000000000000002 000000001b010000 0000000000000000 
[ 8074.351177] GPR20: 000000003bc24df7 c0000000e74fbc28 0000000000000049 0000000000000052 
[ 8074.351177] GPR24: 000000000000002d c0000000ffe30780 c0000008a991d800 000000000000002d 
[ 8074.351177] GPR28: 0000000000000069 0000000000000000 c0000000ffe30780 c0000008a991d800 
[ 8074.351224] NIP [c0000000004ddf30] request_key_auth_describe+0x90/0xd0
[ 8074.351230] LR [c0000000004ddef4] request_key_auth_describe+0x54/0xd0
[ 8074.351233] Call Trace:
[ 8074.351237] [c0000000e74fba90] [c0000000004ddef4] request_key_auth_describe+0x54/0xd0 (unreliable)
[ 8074.351244] [c0000000e74fbb10] [c0000000004df718] proc_keys_show+0x308/0x4c0
[ 8074.351250] [c0000000e74fbcc0] [c000000000404950] seq_read+0x3d0/0x540
[ 8074.351255] [c0000000e74fbd40] [c0000000004865e0] proc_reg_read+0x90/0x110
[ 8074.351261] [c0000000e74fbd70] [c0000000003c901c] __vfs_read+0x3c/0x70
[ 8074.351267] [c0000000e74fbd90] [c0000000003c9104] vfs_read+0xb4/0x1b0
[ 8074.351272] [c0000000e74fbdd0] [c0000000003c95ec] ksys_read+0x7c/0x130
[ 8074.351277] [c0000000e74fbe20] [c00000000000b388] system_call+0x5c/0x70
[ 8074.351281] Instruction dump:
[ 8074.351285] 2b890001 419e002c 38210080 e8010010 eba1ffe8 ebc1fff0 ebe1fff8 7c0803a6 
[ 8074.351292] 4e800020 60000000 60000000 60420000 <e8bd003a> e8dd0030 3c82ff93 7fc3f378 
[ 8074.351301] ---[ end trace d3304a3a5a0a0ca1 ]—

These CVE tests from LTP were recently added to the automated regression test bucket that
I run against upstream. I can’t tell if this is a regression or a new problem.

Thanks
-Sachin