Bug 1378600

Summary: rpc.gssd crash while dereferencing null pointer in gssi_release_name
Product: Red Hat Enterprise Linux 7 Reporter: Frank Sorenson <fsorenso>
Component: gssproxyAssignee: Robbie Harwood <rharwood>
Status: CLOSED ERRATA QA Contact: Yongcheng Yang <yoyang>
Severity: medium Docs Contact:
Priority: medium    
Version: 7.3CC: dpal, eguan, fs-qe, jiyin, ksiddiqu, yoyang
Target Milestone: rc   
Target Release: ---   
Hardware: All   
OS: Linux   
URL: https://pagure.io/gssproxy/pull-request/32
Whiteboard:
Fixed In Version: gssproxy-0.6.2-4.el7 Doc Type: Bug Fix
Doc Text:
Cause: Explicit null dereference in a gssapi function handler. Consequence: rpc.gssd could unexpectedly crash in the gssproxy mechglue. Fix: We tolerate a null pointer there as per specification. Result: No more crash.
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-08-01 20:55:26 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On:    
Bug Blocks: 1399979    
Attachments:
Description Flags
core from rpc.gssd
none
patch none

Description Frank Sorenson 2016-09-22 21:06:06 UTC
Created attachment 1203910 [details]
core from rpc.gssd

Description of problem:

gssi_inquire_context can call into gssi_release_name with input_name being a null pointer, resulting in a crash when it gets dereferenced.


Version-Release number of selected component (if applicable):

kernel-3.10.0-510.el7.x86_64
nfs-utils-1.3.0-0.33.el7.x86_64
gssproxy-0.4.1-13.el7.x86_64


How reproducible:

easily


Steps to Reproduce:

with an nfs mount using sec=krb5, obtain a ticket with a very short lifetime, then access the mount...  repeat several times

nfs client system is vm1, server is vm17

# klist -c /tmp/krb5ccmachine_SORENSON.REDHAT.COM 
Ticket cache: FILE:/tmp/krb5ccmachine_SORENSON.REDHAT.COM
Default principal: nfs/vm1.sorenson.redhat.com.COM

Valid starting       Expires              Service principal
09/22/2016 14:09:58  09/22/2016 18:09:58  krbtgt/SORENSON.REDHAT.COM.COM
	renew until 09/23/2016 14:09:58
09/22/2016 14:09:58  09/22/2016 18:09:58  nfs/vm17.sorenson.redhat.com.COM

vm17:/exports3 on /mnt/credcache type nfs (rw,relatime,vers=3,rsize=262144,wsize=262144,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=krb5,mountaddr=192.168.122.85,mountvers=3,mountport=56413,mountproto=udp,local_lock=none,addr=192.168.122.85)

# su - user1
[user1@vm1 ~]$ $ while true ; do echo 'PASS' | kinit -l 1s user1.COM ; /bin/ls -1fl /mnt/credcache/ >/dev/null && echo OK ; sleep 2 ; done


Actual results:

$ while true ; do echo 'PASS' | kinit -l 1s user1.COM ; /bin/ls -1fl /mnt/credcache/ >/dev/null && echo OK ; sleep 2 ; done
Password for user1.COM: 
OK
Password for user1.COM: 
OK
Password for user1.COM: 
OK
Password for user1.COM: 
OK
Password for user1.COM: 
OK
Password for user1.COM: 
/bin/ls: cannot access /mnt/credcache/foo: Permission denied
/bin/ls: cannot access /mnt/credcache/bar: Permission denied
/bin/ls: reading directory /mnt/credcache/: Permission denied
Password for user1.COM: 
/bin/ls: cannot access /mnt/credcache/: Permission denied
Password for user1.COM: 


Sep 22 14:33:51 vm1 kernel: rpc.gssd[11608]: segfault at 10 ip 00007fd15ee4a946 sp 00007fd15ea04b00 error 4 in proxymech.so[7fd15ee39000+18000]

After the rpc.gssd thread crashes, rpc.gssd must be restarted


Expected results:

no crash


Additional info:

backtrace:

#0  gssi_release_name (minor_status=minor_status@entry=0x7fd3ed3aab9c, input_name=input_name@entry=0x7fd3ed3aaba0)
    at src/mechglue/gpp_import_and_canon_name.c:279
#1  0x00007fd3ed7ef99a in gssi_inquire_context (minor_status=0x7fd3ed3aace8, context_handle=0x7fd3e8029e20, src_name=0x0, 
    targ_name=0x7fd3ed3aac48, lifetime_rec=<optimized out>, mech_type=0x7fd3ed3aac40, ctx_flags=0x0, locally_initiated=0x0, open=0x0)
    at src/mechglue/gpp_context.c:278
#2  0x00007fd3ef819a3f in gss_inquire_context (minor_status=minor_status@entry=0x7fd3ed3aace8, context_handle=<optimized out>, 
    src_name=src_name@entry=0x0, targ_name=targ_name@entry=0x7fd3ed3aacf8, lifetime_rec=lifetime_rec@entry=0x7fd3ed3aacec, 
    mech_type=mech_type@entry=0x7fd3ed3aad00, ctx_flags=ctx_flags@entry=0x0, locally_initiated=locally_initiated@entry=0x0, 
    opened=opened@entry=0x0) at g_inq_context.c:114
#3  0x00007fd3efebf560 in process_krb5_upcall (clp=clp@entry=0x7fd3f16122d0, uid=uid@entry=500, fd=13, tgtname=tgtname@entry=0x0, 
    service=service@entry=0x0) at gssd_proc.c:670
#4  0x00007fd3efebfbaf in handle_gssd_upcall (info=0x7fd3f1609be0) at gssd_proc.c:803
#5  0x00007fd3eec9cdc5 in start_thread (arg=0x7fd3ed3ab700) at pthread_create.c:308
#6  0x00007fd3ee9ca21d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113


The crash occurs when accessing name->local in gssi_release_name, called from gssi_inquire_context

#0  gssi_release_name (minor_status=minor_status@entry=0x7fd3ed3aab9c, input_name=input_name@entry=0x7fd3ed3aaba0)
    at src/mechglue/gpp_import_and_canon_name.c:279
279	    if (!name->local && !name->remote) {

(gdb) p input_name
$3 = (gss_name_t *) 0x7fd3ed3aaba0

(gdb) p *input_name
$4 = (gss_name_t) 0x0


src_name is 0x0 by the time we get into gss_inquire_context in frames 1 & 2:
#1  0x00007fd3ed7ef99a in gssi_inquire_context (..., src_name=0x0, ...)
    at src/mechglue/gpp_context.c:278

#2  0x00007fd3ef819a3f in gss_inquire_context (..., src_name=src_name@entry=0x0, ...) at g_inq_context.c:114

Comment 2 Frank Sorenson 2016-09-22 21:20:46 UTC
If either src_name or targ_name is NULL to start out with, or if one of the memory allocations fails, the s_name or t_name will also be NULL when reaching the bottom of the function.

There are several places in gssi_inquire_context where src_name and targ_name are checked, but not when freeing s_name or t_name.


OM_uint32 gssi_inquire_context(OM_uint32 *minor_status,
{
    struct gpp_name_handle *s_name = NULL;
    struct gpp_name_handle *t_name = NULL;

    if (src_name) {
        s_name = calloc(1, sizeof(struct gpp_name_handle));
        if (!s_name) {
            min = ENOMEM;
            maj = GSS_S_FAILURE;
            goto done;
        }
    }
    if (targ_name) {
        t_name = calloc(1, sizeof(struct gpp_name_handle));
        if (!t_name) {
            min = ENOMEM;
            maj = GSS_S_FAILURE;
            goto done;
        }
    }
...

    if (maj != GSS_S_COMPLETE) {
        goto done;
    }

    if (s_name) {
...
    }

    if (t_name) {
...
    }
...

done:
    if (maj == GSS_S_COMPLETE) {
...
    } else {
        (void)gss_release_oid(&min, &mech_oid);
        (void)gssi_release_name(&min, (gss_name_t *)&s_name);
        (void)gssi_release_name(&min, (gss_name_t *)&t_name);

Comment 3 Frank Sorenson 2016-09-22 21:30:30 UTC
Created attachment 1203911 [details]
patch

There are obviously other ways to fix this, but here's a patch which prevents the segfault.

Comment 4 Robbie Harwood 2016-09-23 18:45:38 UTC
You're absolutely right.  I'm somewhat surprised static analysis tools don't catch this.  Thanks for the great bug report.

I'm proposing a (slightly) more broad patch upstream.

Comment 8 Yongcheng Yang 2017-05-25 11:42:44 UTC
Moving to VERIFIED according to test logs of Comment #7

Comment 9 errata-xmlrpc 2017-08-01 20:55:26 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2017:2033