[Bug runtime/20742] kernel pointer dereference BUG on RHEL6 s390x

["dsmith at redhat dot com" <sourceware-bugzilla at sourceware dot org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=20742

--- Comment #1 from David Smith <dsmith at redhat dot com> ---
Created attachment 9606
  --> https://sourceware.org/bugzilla/attachment.cgi?id=9606&action=edit
proposed patch

Here's a patch that attempts to fix this problem. There are actually 3 fixes in
this patch.

Fix #1: uprobe_report_clone(), did the following:

====
        rcu_read_lock();
        ptask = (struct uprobe_task *)rcu_dereference(engine->data);
        uproc = ptask->uproc;
        rcu_read_unlock();

        /*
         * Lock uproc so no new uprobes can be installed 'til all
         * report_clone activities are completed.  Lock uproc_table
         * in case we have to run uprobe_fork_uproc().
         */
        lock_uproc_table();
        down_write(&uproc->rwsem);
====

This isn't correct. When we're under the rcu_read_lock(), the ptask and uproc
points should be valid. However, outside of the rcu protection, the ptask/uproc
memory can be freed.

To fix this, I changed the code to the following:

====
        rcu_read_lock();
        ptask = (struct uprobe_task *)rcu_dereference(engine->data);
        BUG_ON(!ptask);
        /* Keep uproc intact until just before we return. */
        uproc = uprobe_get_process(ptask->uproc);
        rcu_read_unlock();

        if (!uproc)
                /* uprobe_free_process() has probably clobbered utask->proc. */
                return UTRACE_DETACH;

        /*
         * Lock uproc so no new uprobes can be installed 'til all
         * report_clone activities are completed.  Lock uproc_table
         * in case we have to run uprobe_fork_uproc().
         */
        lock_uproc_table();
        down_write(&uproc->rwsem);
====

This changes the code to increase the reference count on uproc so it won't get
freed. Similar code is present in uprobe_report_signal(), uprobe_report_exit(),
etc.

I also added code to uprobe_report_clone() to decrement the reference count on
uproc before returning.

When the systemtap.unprivileged/pr16806.exp is run with the
uprobe_report_clone() changes, the test no longer crashed the system, but 2 new
problems popped up:

[ BUG: lock held when returning to user space! ]                                
------------------------------------------------                                
loop/4866 is leaving the kernel with locks still held!                          
2 locks held by loop/4866:                                                      
 #0:  (&uproc->rwsem){+++++.}, at: [<000003e00108ad32>]
uprobe_report_signal+0x2
96/0xe80 [uprobes]                                                              
 #1:  (&slot->rwsem){+.+...}, at: [<000003e0010898e0>]
uprobe_find_insn_slot+0x1
60/0x33c [uprobes]                                                              


Fix #2: To fix first lock being held problem above I took a look at
uprobe_report_signal(). That code is quit complicated, and has 'goto'
statements that jump from one switch statement case to another. I could see
several paths through that code that could lead to uproc->rwsem still being
held on exit.

The changes to uprobe_report_signal() attempt to fix those issues.


Fix #3: uprobe_find_insn_slot() always returns the slot read-locked. It is only
called by uprobe_get_insn_slot(), which returns that read-locked slot.
uprobe_get_insn_slot() is only called by uprobe_pre_ssout(), which wasn't
unlocking the slot. I changed uprobe_pre_ssout() to unlock the slot before
returning.


With this path the test passes fine. However, I'll need to do more testing to
ensure I haven't broken anything else.

-- 
You are receiving this mail because:
You are the assignee for the bug.