This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [PATCH] uprobes: register_uprobe() crashes when bailing out.
- From: Torsten Polle <Torsten dot Polle at gmx dot de>
- To: systemtap at sourceware dot org
- Date: Sat, 13 Apr 2013 13:18:56 +0200
- Subject: Re: [PATCH] uprobes: register_uprobe() crashes when bailing out.
- References: <m2mwt62k7r dot fsf at gmx dot de> <y0mhajbnvfs dot fsf at fche dot csb>
Hi Frank,
Frank Ch Eigler writes:
> Hi -
> Torsten.Polle wrote:
>> [...]
>> Subject: [PATCH] uprobes: register_uprobe() crashes when bailing out.
>>
>> uprobe_mk_process() initialises uproc->hlist, but does not put uproc
>> on any list, i.e. uproc_table. If register_uprobe() now bails out
>> before uproc is put on a list, uprobe_free_process() still tries to
>> remove uproc from a list. But hlist_del() only works, if the element
>> is already on list. hlist_del_init() first checks if the element is
>> on any list, before it removes the element (uproc) from the list.
> That description doesn't sound quite right to me. I could be
> mistaken, but I thought the hlist_del_init() variant was available so
> that the hlist pointers are cleared after deletion (for purposes of
> reuse in a different list perhaps), not in order to be checked for
> list-membership before deletion. This The change may still be right,
> but perhaps for a different reason.
below is the flow of events in this case. I have an issue with a probe
definition which makes register_uprobe() bail out. (I will follow up on
the that issue after I'm through with the analysis.)
register_uprobe() is called.
...
uproc = uprobe_mk_process(p, 0);
INIT_HLIST_NODE(&uproc->hlist);
h->next = NULL;
h->pprev = NULL;
uproc_is_new = 1;
Now uproc->hlist.pprev is NULL.
I know why register_uprobe() bails out but not where. I only know that
goto fail_uproc;
is executed.
Therefore uprobe_free_process() is called. This in turn does the following.
hlist_del(&uproc->hlist);
__hlist_del(n);
struct hlist_node *next = n->next;
struct hlist_node **pprev = n->pprev;
*pprev = next;
As n->pprev is still NULL, *pprev leads to a NULL pointer exception.
The reason why I chose hlist_del_init() is that it first checks for
pprev to be not NULL by calling hlist_unhashed().
static inline void hlist_del_init(struct hlist_node *n)
{
if (!hlist_unhashed(n)) {
__hlist_del(n);
INIT_HLIST_NODE(n);
}
}
My idea was to either call hlist_unhashed() in uprobe_free_process() or
let hlist_del_init() do the job.
Kind Regards,
Torsten