This small patchset fixes a kernel crash when the rpcgss_context trace event is
enabled and IO is in flight when a kerberos ticket expires. The crash occurs
because the acceptor name may be 0 bytes long and the gss_fill_context() function
does not handle it properly. This causes the ctx->gc_acceptor.data to be
ZERO_SIZE_PTR which is not properly recognized by the tracepoint code.
The first patch is a simple refactor and eliminates duplicate helper functions
related to the crash. The second patch is the actual fix inside one of the
helper functions due to the definition of an opaque XDR object. This object
is defined in RFC 4506 (see section 4.10), where 'length' is an integer in a
range including 0.
Reproducer
# Enable the tracepoint and mount the share
trace-cmd start -e rpcgss:*
mount -osec=krb5 nfs-server:/export /mnt/nfs
# Obtain a kerberos ticket
# Set ticket lifetime to something small like 20 seconds
su test -c "kinit -l 20 test"
# Sleep for a portion of the ticket lifetime so we are writing while the ticket expires
sleep 10
# Now run some IO long enough that the ticket expires midway
dd if=/dev/urandom of=/mnt/nfs/file bs=1M count=100
Dave Wysochanski (2):
SUNRPC: Move simple_get_bytes and simple_get_netobj into xdr.h
SUNRPC: Handle 0 length opaque XDR object data properly
include/linux/sunrpc/xdr.h | 33 +++++++++++++++++++++++++++--
net/sunrpc/auth_gss/auth_gss.c | 29 -------------------------
net/sunrpc/auth_gss/gss_krb5_mech.c | 29 -------------------------
3 files changed, 31 insertions(+), 60 deletions(-)
--
2.25.2
When handling an auth_gss downcall, it's possible to get 0-length
opaque object for the acceptor. In this case we should make sure
simple_get_netobj() returns -EFAULT and does not continue to
kmemdup() which will return a special pointer value of
ZERO_SIZE_PTR. If it just so happens that the rpcgss_context
trace event is enabled, the kernel will crash as follows:
[ 162.887992] BUG: kernel NULL pointer dereference, address: 0000000000000010
[ 162.898693] #PF: supervisor read access in kernel mode
[ 162.900830] #PF: error_code(0x0000) - not-present page
[ 162.902940] PGD 0 P4D 0
[ 162.904027] Oops: 0000 [#1] SMP PTI
[ 162.905493] CPU: 4 PID: 4321 Comm: rpc.gssd Kdump: loaded Not tainted 5.10.0 #133
[ 162.908548] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
[ 162.910978] RIP: 0010:strlen+0x0/0x20
[ 162.912505] Code: 48 89 f9 74 09 48 83 c1 01 80 39 00 75 f7 31 d2 44 0f b6 04 16 44 88 04 11 48 83 c2 01 45 84 c0 75 ee c3 0f 1f 80 00 00 00 00 <80> 3f 00 74 10 48 89 f8 48 83 c0 01 80 38 00 75 f7 48 29 f8 c3 31
[ 162.920101] RSP: 0018:ffffaec900c77d90 EFLAGS: 00010202
[ 162.922263] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 00000000fffde697
[ 162.925158] RDX: 000000000000002f RSI: 0000000000000080 RDI: 0000000000000010
[ 162.928073] RBP: 0000000000000010 R08: 0000000000000e10 R09: 0000000000000000
[ 162.930976] R10: ffff8e698a590cb8 R11: 0000000000000001 R12: 0000000000000e10
[ 162.933883] R13: 00000000fffde697 R14: 000000010034d517 R15: 0000000000070028
[ 162.936777] FS: 00007f1e1eb93700(0000) GS:ffff8e6ab7d00000(0000) knlGS:0000000000000000
[ 162.940067] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 162.942417] CR2: 0000000000000010 CR3: 0000000104eba000 CR4: 00000000000406e0
[ 162.945300] Call Trace:
[ 162.946428] trace_event_raw_event_rpcgss_context+0x84/0x140 [auth_rpcgss]
[ 162.949308] ? __kmalloc_track_caller+0x35/0x5a0
[ 162.951224] ? gss_pipe_downcall+0x3a3/0x6a0 [auth_rpcgss]
[ 162.953484] gss_pipe_downcall+0x585/0x6a0 [auth_rpcgss]
[ 162.955953] rpc_pipe_write+0x58/0x70 [sunrpc]
[ 162.957849] vfs_write+0xcb/0x2c0
[ 162.959264] ksys_write+0x68/0xe0
[ 162.960706] do_syscall_64+0x33/0x40
[ 162.962238] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 162.964346] RIP: 0033:0x7f1e1f1e57df
Signed-off-by: Dave Wysochanski <[email protected]>
---
include/linux/sunrpc/xdr.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
index 8ef788ff80b9..aca38bdb0059 100644
--- a/include/linux/sunrpc/xdr.h
+++ b/include/linux/sunrpc/xdr.h
@@ -53,7 +53,7 @@ simple_get_netobj(const void *p, const void *end, struct xdr_netobj *dest)
if (IS_ERR(p))
return p;
q = (const void *)((const char *)p + len);
- if (unlikely(q > end || q < p))
+ if (unlikely(q > end || q <= p))
return ERR_PTR(-EFAULT);
dest->data = kmemdup(p, len, GFP_NOFS);
if (unlikely(dest->data == NULL))
--
2.25.2
On Tue, Jan 5, 2021 at 5:12 PM Dave Wysochanski <[email protected]> wrote:
>
> When handling an auth_gss downcall, it's possible to get 0-length
> opaque object for the acceptor. In this case we should make sure
> simple_get_netobj() returns -EFAULT and does not continue to
> kmemdup() which will return a special pointer value of
> ZERO_SIZE_PTR. If it just so happens that the rpcgss_context
> trace event is enabled, the kernel will crash as follows:
>
> [ 162.887992] BUG: kernel NULL pointer dereference, address: 0000000000000010
> [ 162.898693] #PF: supervisor read access in kernel mode
> [ 162.900830] #PF: error_code(0x0000) - not-present page
> [ 162.902940] PGD 0 P4D 0
> [ 162.904027] Oops: 0000 [#1] SMP PTI
> [ 162.905493] CPU: 4 PID: 4321 Comm: rpc.gssd Kdump: loaded Not tainted 5.10.0 #133
> [ 162.908548] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
> [ 162.910978] RIP: 0010:strlen+0x0/0x20
> [ 162.912505] Code: 48 89 f9 74 09 48 83 c1 01 80 39 00 75 f7 31 d2 44 0f b6 04 16 44 88 04 11 48 83 c2 01 45 84 c0 75 ee c3 0f 1f 80 00 00 00 00 <80> 3f 00 74 10 48 89 f8 48 83 c0 01 80 38 00 75 f7 48 29 f8 c3 31
> [ 162.920101] RSP: 0018:ffffaec900c77d90 EFLAGS: 00010202
> [ 162.922263] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 00000000fffde697
> [ 162.925158] RDX: 000000000000002f RSI: 0000000000000080 RDI: 0000000000000010
> [ 162.928073] RBP: 0000000000000010 R08: 0000000000000e10 R09: 0000000000000000
> [ 162.930976] R10: ffff8e698a590cb8 R11: 0000000000000001 R12: 0000000000000e10
> [ 162.933883] R13: 00000000fffde697 R14: 000000010034d517 R15: 0000000000070028
> [ 162.936777] FS: 00007f1e1eb93700(0000) GS:ffff8e6ab7d00000(0000) knlGS:0000000000000000
> [ 162.940067] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 162.942417] CR2: 0000000000000010 CR3: 0000000104eba000 CR4: 00000000000406e0
> [ 162.945300] Call Trace:
> [ 162.946428] trace_event_raw_event_rpcgss_context+0x84/0x140 [auth_rpcgss]
> [ 162.949308] ? __kmalloc_track_caller+0x35/0x5a0
> [ 162.951224] ? gss_pipe_downcall+0x3a3/0x6a0 [auth_rpcgss]
> [ 162.953484] gss_pipe_downcall+0x585/0x6a0 [auth_rpcgss]
> [ 162.955953] rpc_pipe_write+0x58/0x70 [sunrpc]
> [ 162.957849] vfs_write+0xcb/0x2c0
> [ 162.959264] ksys_write+0x68/0xe0
> [ 162.960706] do_syscall_64+0x33/0x40
> [ 162.962238] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [ 162.964346] RIP: 0033:0x7f1e1f1e57df
>
> Signed-off-by: Dave Wysochanski <[email protected]>
> ---
> include/linux/sunrpc/xdr.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
> index 8ef788ff80b9..aca38bdb0059 100644
> --- a/include/linux/sunrpc/xdr.h
> +++ b/include/linux/sunrpc/xdr.h
> @@ -53,7 +53,7 @@ simple_get_netobj(const void *p, const void *end, struct xdr_netobj *dest)
> if (IS_ERR(p))
> return p;
> q = (const void *)((const char *)p + len);
> - if (unlikely(q > end || q < p))
> + if (unlikely(q > end || q <= p))
> return ERR_PTR(-EFAULT);
> dest->data = kmemdup(p, len, GFP_NOFS);
> if (unlikely(dest->data == NULL))
> --
> 2.25.2
>
Just following up on this as I got no response, but didn't see
it in anyone's tree yet.
After talking offline with Bruce about this, I think the above
approach is wrong and i'm going to do a v2 of this patch.
The short reason is, I don't think simple_get_netobj() should be
returning the -EFAULT in the case of a 0-length acceptor,
which I explain in the cover letter why that is valid as an
object. Rather I think this patch should do a explicit check
for 0 length, and then fill in dest->data = NULL:
diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
index 8ef788ff80b9..b4f5bf104405 100644
--- a/include/linux/sunrpc/xdr.h
+++ b/include/linux/sunrpc/xdr.h
@@ -55,9 +55,12 @@ struct xdr_netobj {
q = (const void *)((const char *)p + len);
if (unlikely(q > end || q < p))
return ERR_PTR(-EFAULT);
- dest->data = kmemdup(p, len, GFP_NOFS);
- if (unlikely(dest->data == NULL))
- return ERR_PTR(-ENOMEM);
+ if (len) {
+ dest->data = kmemdup(p, len, GFP_NOFS);
+ if (unlikely(dest->data == NULL))
+ return ERR_PTR(-ENOMEM);
+ } else
+ dest->data = NULL;
dest->len = len;
return q;