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

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

Can you try this patch instead of Hillf's?

David
---
commit df882ad6d4e24a3763719c1798ea58e87d56c2d7
Author: Hillf Danton [off-list ref]
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 [off-list ref]
    Signed-off-by: David Howells [off-list ref]

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;
 

On 30-Aug-2019, at 8:43 PM, David Howells [off-list ref] wrote:

Can you try this patch instead of Hillf’s?

Works for me. Test ran fine without any problem.

Tested-by: Sachin Sant <redacted>

Thanks
-Sachin

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 [off-list ref]
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 [off-list ref]
    Signed-off-by: David Howells [off-list ref]
    Tested-by: Sachin Sant [off-list ref]

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;
 

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.

Signed-off-by: Hillf Danton <redacted>