[ltt-dev] [PATCH] lttng: Fix potential invalid pointer derefference

[guijianfeng@cn.fujitsu.com (Gui Jianfeng)]

Mathieu Desnoyers wrote:
> * Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
>> Mathieu Desnoyers wrote:
>>> * Gui Jianfeng (guijianfeng at cn.fujitsu.com) wrote:
>>>> Hi Mathieu,
>>>>
>>>> Calling get_ltt_root() and put_ltt_root() pair may induce invalid pointer
>>>> derefference. Consider the following scenario:
>>>>
>>>>          CPU 0                   CPU 1
>>>>   ----------------------  ----------------------
>>>> 1  root = get_ltt_root()   
>>>> 2                          root = get_ltt_root()
>>>> 3                          put_ltt_root() //global root is freed
>>>> 4  using root(crash here)
>>>>
>>>> Here is the fix for this issue. Thanks a lot to Zhaolei to point this out
>>>> in offline.
>>>>
>>> Agreed, there is a problem. Please see comments below,
>>>
>>>> This patch assumes that the *ltt root remove* patch has been applied.
>>>>
>>>> Signed-off-by: Gui Jianfeng <guijianfeng at cn.fujistu.com>
>>>> ---
>>>>  include/linux/ltt-core.h  |    8 +++++++-
>>>>  ltt/ltt-core.c            |   38 +++++++++++++++++++++++++++-----------
>>> Only modifications to the two files above seems justified. The rest
>>> could probably stay as-is.
>>>
>>>>  ltt/ltt-relay.c           |    7 ++++++-
>>>>  ltt/ltt-trace-control.c   |   13 +++++++------
>>>>  ltt/ltt-userspace-event.c |   12 ++++++++----
>>>>  5 files changed, 55 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/include/linux/ltt-core.h b/include/linux/ltt-core.h
>>>> index 28394cb..4e20f78 100644
>>>> --- a/include/linux/ltt-core.h
>>>> +++ b/include/linux/ltt-core.h
>>>> @@ -27,11 +27,17 @@ struct ltt_traces {
>>>>  
>>>>  extern struct ltt_traces ltt_traces;
>>>>  
>>>> +/* ltt's root dir */
>>>> +struct ltt_root {
>>>> +	atomic_t ref;
>>>> +	struct dentry *root;
>>>> +};
>>>> +
>>> Please use include/linux/kref.h.
>>   Do you mean this struct should move into ltt-core.c as following:
>>   struct ltt_root {
>> 	struct dentry *root;
>> 	struct kref ref;
>>   };
>>   and get_ltt_root() will still return a dentry pointer?
>>
> 
> Or even stay as
> 
> struct dentry *root;
> struct kref root_dentry_kref;
> 
> There is no real need for a structure around them.

  See, will post v2

> 
> Mathieu


-- 
Regards
Gui Jianfeng