Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Song Liu <songliubraving@fb.com>
To:     Steven Rostedt <rostedt@goodmis.org>
CC:     Miklos Szeredi <miklos@szeredi.hu>,
        LKML <linux-kernel@vger.kernel.org>,
        Kernel Team <Kernel-team@fb.com>,
        Ingo Molnar <mingo@redhat.com>,
        "Howard McLauchlan" <hmclauchlan@fb.com>,
        Josef Bacik <jbacik@fb.com>,
        "Srikar Dronamraju" <srikar@linux.vnet.ibm.com>
Subject: Re: [PATCH 1/2] tracing: fix bad use of igrab in trace_uprobe.c
Thread-Topic: [PATCH 1/2] tracing: fix bad use of igrab in trace_uprobe.c
Thread-Index: AQHT1t6qY2w++l1Em02GF9oER/KoaKQGjmAAgAAF+ACAAB7zAA==
Date:   Wed, 18 Apr 2018 16:15:50 +0000
Message-ID: <939B1EB1-07D3-473F-B7D1-21E7B0F6CF13@fb.com>
References: <20180418062907.3210386-1-songliubraving@fb.com>
 <CAELBmZCvrL4s+cx2cECMQLQUx0hMxVP-dv9UGBfh64AS6unaBA@mail.gmail.com>
 <20180418102504.7673a7f3@gandalf.local.home>
In-Reply-To: <20180418102504.7673a7f3@gandalf.local.home>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1CB3466784AB09459DA4CC3625D04484@namprd15.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: f005e8bd-b8f0-4811-47ce-08d5a547a73c
X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Apr 2018 16:15:50.9174
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR15MB1611
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-04-18_03:,,
 signatures=0
X-Proofpoint-Spam-Reason: safe
X-FB-Internal: Safe
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> On Apr 18, 2018, at 7:25 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>=20
> On Wed, 18 Apr 2018 16:03:42 +0200
> Miklos Szeredi <miklos@szeredi.hu> wrote:
>=20
>>> @@ -937,7 +928,8 @@ probe_event_enable(struct trace_uprobe *tu, struct =
trace_event_file *file,
>>>                goto err_flags;
>>>=20
>>>        tu->consumer.filter =3D filter;
>>> -       ret =3D uprobe_register(tu->inode, tu->offset, &tu->consumer);
>>> +       ret =3D uprobe_register(d_inode(tu->path.dentry), tu->offset,
>>> +                             &tu->consumer); =20
>>=20
>> It is not entirely clear how the lifetime of uprobe relates to the
>> lifetime of trace_uprobe.  Is the uprobe object never going to survive
>> its creator trace_uprobe object?
>=20
> Not exactly sure what you mean here.
>=20
> The trace_uprobe (the probe event) is created, it doesn't do anything
> until it is enabled. This function is called when it is enabled. The
> trace_uprobe (probe event) can not be deleted while it is enabled
> (EBUSY).
>=20
> Are you asking what happens if the file is deleted while it has probe?
> That I don't know about (haven't tried it out). But I would hope that
> it keeps a reference to the inode, isn't that what the igrab is for?
> And is now being replaced by a reference on the path, or is that the
> problem?
>=20
> -- Steve
>=20

Just as Miklos pointed out, I run tests with the uprobe and confirmed=20
that igrab() is not sufficient to prevent umount. When we change it to=20
path_get()/path_put(), umount will abort because of the trace_uprobe.=20

Song


>>=20
>> If that's the case, it warrants a comment.  If that's not the case,
>> then the path would need to be passed to uprobe_resister() which would
>> need to obtain its own reference.
>>=20
>>>        if (ret)
>>>                goto err_buffer;
>>>=20