lkml.org 
[lkml]   [2018]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2] tracing: fix bad use of igrab in trace_uprobe.c
On Thu, Apr 19, 2018 at 6:37 PM, Song Liu <songliubraving@fb.com> wrote:
>
>
>> On Apr 19, 2018, at 7:44 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>
>> On Thu, Apr 19, 2018 at 10:58 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>> On Wed, Apr 18, 2018 at 7:40 PM, Song Liu <songliubraving@fb.com> wrote:

>>>> *arg++ = '\0';
>>>> filename = argv[1];
>>>> ret = kern_path(filename, LOOKUP_FOLLOW, &path);
>>>> if (ret)
>>>> - goto fail_address_parse;
>>>> -
>>>> - inode = igrab(d_real_inode(path.dentry));
>>
>> Also, where has the d_real_inode() gone?
>>
>> Looks like we need tu->inode back, since the return value of
>> d_real_inode() may change over time. I'd do the "tu->inode =
>> d_real_inode(tu->path.dentry)" just before first use (i.e. when
>> enabling the tracepoint).
>>

>
> Do we need mechanism to prevent the return value of d_real_inode()
> to change? Would the following sequence happen?
>
> create trace_uprobe
> enable trace_uprobe (uprobe_register)
> d_real changes
> disable trace_uprobe (uprobe_unregister get wrong inode?)

Yes.

>
> Another case might be:
>
> create trace_uprobe
> enable trace_uprobe (uprobe_register)
> disable trace_uprobe (uprobe_unregister)
> d_real changes
> enable trace_uprobe (do we need new inode for uprobe_register)

Probably a good idea to use the new one, but doesn't really matter.
Do the one that's simpler.

This corner case is simply not interesting (modifying a binary while
it is being debugged with uprobe). Let's just concentrate on making
this crash and leak free.

Thanks,
Miklos

\
 
 \ /
  Last update: 2018-04-20 16:35    [W:0.300 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site