From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 43671 invoked by alias); 1 Nov 2016 20:15:23 -0000 Mailing-List: contact systemtap-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: systemtap-owner@sourceware.org Received: (qmail 43199 invoked by uid 48); 1 Nov 2016 20:14:58 -0000 From: "dsmith at redhat dot com" To: systemtap@sourceware.org Subject: [Bug runtime/20742] kernel pointer dereference BUG on RHEL6 s390x Date: Tue, 01 Nov 2016 20:15:00 -0000 X-Bugzilla-Reason: AssignedTo X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: systemtap X-Bugzilla-Component: runtime X-Bugzilla-Version: unspecified X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: dsmith at redhat dot com X-Bugzilla-Status: NEW X-Bugzilla-Resolution: X-Bugzilla-Priority: P2 X-Bugzilla-Assigned-To: systemtap at sourceware dot org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: attachments.created Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://sourceware.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-SW-Source: 2016-q4/txt/msg00034.txt.bz2 https://sourceware.org/bugzilla/show_bug.cgi?id=3D20742 --- Comment #1 from David Smith --- Created attachment 9606 --> https://sourceware.org/bugzilla/attachment.cgi?id=3D9606&action=3Dedit proposed patch Here's a patch that attempts to fix this problem. There are actually 3 fixe= s in this patch. Fix #1: uprobe_report_clone(), did the following: =3D=3D=3D=3D rcu_read_lock(); ptask =3D (struct uprobe_task *)rcu_dereference(engine->data); uproc =3D 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); =3D=3D=3D=3D This isn't correct. When we're under the rcu_read_lock(), the ptask and upr= oc points should be valid. However, outside of the rcu protection, the ptask/u= proc memory can be freed. To fix this, I changed the code to the following: =3D=3D=3D=3D rcu_read_lock(); ptask =3D (struct uprobe_task *)rcu_dereference(engine->data); BUG_ON(!ptask); /* Keep uproc intact until just before we return. */ uproc =3D 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); =3D=3D=3D=3D 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_exi= t(), 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! ]=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20 ------------------------------------------------=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20 loop/4866 is leaving the kernel with locks still held!=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20 2 locks held by loop/4866:=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20 #0: (&uproc->rwsem){+++++.}, at: [<000003e00108ad32>] uprobe_report_signal+0x2 96/0xe80 [uprobes]=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20 #1: (&slot->rwsem){+.+...}, at: [<000003e0010898e0>] uprobe_find_insn_slot+0x1 60/0x33c [uprobes]=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20 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. --=20 You are receiving this mail because: You are the assignee for the bug.