This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] uprobes: register_uprobe() crashes when bailing out.


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]