Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Trond Myklebust <trondmy@hammerspace.com>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v2 2/2] SUNRPC: Fix the svc_deferred_event trace class
Thread-Topic: [PATCH v2 2/2] SUNRPC: Fix the svc_deferred_event trace class
Thread-Index: AQHYSe/bEHUctVBe9E2g94N5iR8qkazjV7AAgAAFsoA=
Date:   Wed, 6 Apr 2022 20:55:22 +0000
Message-ID: <0942D638-0F40-4811-9820-AECAA65D529B@oracle.com>
References: <164926821551.12216.9112595778893638551.stgit@klimt.1015granger.net>
 <164926848846.12216.6872977249610829189.stgit@klimt.1015granger.net>
 <7976abc7a7ea7bbc445256884d0164a1b3ac5bb6.camel@hammerspace.com>
In-Reply-To: <7976abc7a7ea7bbc445256884d0164a1b3ac5bb6.camel@hammerspace.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?LFG4YK/ycPCEYsYwDNV9WZ1Oo9IrwusM9KirWZMnuhfBmDbjXBjz9JEWRnEV?=
 =?us-ascii?Q?YoskAVv+a56fUUT3eXC2xbYPN5rntXxrt3wZpWhAb2Dh/ureRoZXOJ6nFyWi?=
 =?us-ascii?Q?tIwUTf0NPvPCrXGAWIx2oD/8IpPXcoIZPQilCS5rb+3uz3BqrOSILSvOiJsm?=
 =?us-ascii?Q?ataee6gfqsA+2ifhFYKTdXCTHqGMWKYCpOj5DeXqBFX8KNDE03ctHkBVBHli?=
 =?us-ascii?Q?dXPDGS1qPcDtuQUdAG+FuHJDC0eYE534m51NOGxWO4BPD062WM3ftk5j0J8B?=
 =?us-ascii?Q?x/cgWOx+9aBXnk521CDn40GSUydCTuHFw54NVUIBjMvrAWUHf+Ak7toHHDHi?=
 =?us-ascii?Q?xoVveTTlZbvFHYrkf9+XEBS1yCwMm/fgwHtWLh/hqSZl16mL7BgZIIQneczA?=
 =?us-ascii?Q?s9DYJOKviWqnMjE88/6N55gHA9wrfSmbwGDgRfLdgYxcS86Qu9dF84bZhVVl?=
 =?us-ascii?Q?69HYFkwKaL0dtGVrI6yZtf+P4U9DxQ4zVWvmJyFds648JGVyBtSoTrkNbgL5?=
 =?us-ascii?Q?lbnpDZNexUV1R1kAl53W30epIUnekXiUr8o9xqFYiLcisXdq9YHMBLRjO0pj?=
 =?us-ascii?Q?82u15097MzlbWBSRCGIUHcAAkhOADeHnYFz5G2PXswLSA7M+Jf5TtZo+iM1h?=
 =?us-ascii?Q?zUJluRFpgguhn2hNnhWq3J5+TjgKcO5+wNYSY6+24FmN4YxGbxcdE3I0IjU1?=
 =?us-ascii?Q?D4ko2ULoWOpClT1zNCPUoe8jSGYeHDNcWUD/Rf8dNZcJTkpRYJdit58MwOUj?=
 =?us-ascii?Q?M/kk9CQUZLeLaNupmWvDOaR45DhIuk1cG3uuzX3CpjQtizbWVWIZaBrE3y+N?=
 =?us-ascii?Q?nGBugtDFQHq8T10xUZaGazPo8//swlr9JX/GpJqinXH/3PejdCwcqT94vsmj?=
 =?us-ascii?Q?DtB4238XgwIS0nFvxJ1no+gS56jTvazmPgB6blmEa2HNVWwpB9v0hg9L169S?=
 =?us-ascii?Q?e412F17HDz3zoVQbJmWjcBDW4qG84/pbapxJGqbob7YSCz3DbXkHfqMOik6C?=
 =?us-ascii?Q?y+AigC8hSVMyUskQoQApzC/1wfcmBPDIU0k2Nw9XLfYYJCQrcfRlnHyC65E2?=
 =?us-ascii?Q?S4HFSpm09A3hJ27zsNrts/NuSOsVPIrys0ov7TLIFgwKufoi6SuMCtLs3KC7?=
 =?us-ascii?Q?7LE7fy0D+zX1P6FAiGW9sYWd+QncMjAocsoHDyxZBTdu6TEq1kjwNY7fQ+9u?=
 =?us-ascii?Q?YzP5p0stY9Rs9B8nWQL2RK84ni124bCUGNx5jnUlysAs40w4f8fEVdIxEgiO?=
 =?us-ascii?Q?+rMmvXJn4YAUpnGy9JzwgLozQfcdVZ/eOWBYdR0rsh2Zo9CvdM9tArkLO7De?=
 =?us-ascii?Q?Lj00KcYkrajl9c7twAhhDcImrgE59LAtRt4lUZM1J5se15IqJR6++VU7uJGe?=
 =?us-ascii?Q?8iiNT4NJRs19EZRv11fi493urGcX/Hg6cklIFPl8kdBbi+ty/gnpGP2eusLq?=
 =?us-ascii?Q?NudpKeKRhRGvuLk4cgdVOlbH7hVi/mOGwKyBMQVspCUtUAep44I2qWpduhIW?=
 =?us-ascii?Q?1fhPspymz5F1INXU04+g4XvWkRRUG2Tj49dpoGTUTHJEEw0o5QMiqFJuQ8aG?=
 =?us-ascii?Q?iUYsV9WE1RdrTNPM6k7Mpdswe54ejmZ+qXL6ek9yHeOyOskxy02/dUT7Aakn?=
 =?us-ascii?Q?HZPXgg8FrcTLPO1Q0jw8K3fZiIDYHdJIjEZUsiEATGzIfbO7MQ7fWakt/Gio?=
 =?us-ascii?Q?4c6AOPl6OvP0BgFWwy9it8ENfrVXHfpqUD6rk08uh/Ld07bAfdUVSU1qIj/T?=
 =?us-ascii?Q?IpnfNBdfoSSJzxboTq/UpQcpkAg1/eo=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <022CAAE20515F14BB90D7310DA91F0EB@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4ca218f9-6d71-4cbb-3b77-08da180fc48d
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Apr 2022 20:55:22.7029
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: nGQWySJOff+SjKkOygmF4xDYoTnK2I8CHjSydKHr1ZRcz08OfFBh1xg2AKLmHDaqbDBaBaSoIQCeEO6Okr2+CQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR10MB2573
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.425,18.0.850
 definitions=2022-04-06_12:2022-04-06,2022-04-06 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 phishscore=0 malwarescore=0
 mlxlogscore=999 suspectscore=0 adultscore=0 bulkscore=0 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2204060104
X-Proofpoint-GUID: LH9QIb0UOFQinfpSWh1HYBCG6PGaGmkG
X-Proofpoint-ORIG-GUID: LH9QIb0UOFQinfpSWh1HYBCG6PGaGmkG
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Apr 6, 2022, at 4:34 PM, Trond Myklebust <trondmy@hammerspace.com> wro=
te:
>=20
> On Wed, 2022-04-06 at 14:08 -0400, Chuck Lever wrote:
>> Fix a NULL deref crash that occurs when an svc_rqst is deferred
>> while the sunrpc tracing subsystem is enabled. svc_revisit() sets
>> dr->xprt to NULL, so it can't be relied upon in the tracepoint to
>> provide the remote's address.
>>=20
>> Since __sockaddr() and friends are not available before v5.18, this
>> is just a partial revert of commit ece200ddd54b ("sunrpc: Save
>> remote presentation address in svc_xprt for trace events") in order
>> to enable backports of the fix. It can be cleaned up during a
>> future merge window.
>>=20
>> Fixes: ece200ddd54b ("sunrpc: Save remote presentation address in
>> svc_xprt for trace events")
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>>  include/trace/events/sunrpc.h |    9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>=20
>> diff --git a/include/trace/events/sunrpc.h
>> b/include/trace/events/sunrpc.h
>> index ab8ae1f6ba84..4abc2fddd3b8 100644
>> --- a/include/trace/events/sunrpc.h
>> +++ b/include/trace/events/sunrpc.h
>> @@ -2017,18 +2017,19 @@ DECLARE_EVENT_CLASS(svc_deferred_event,
>>         TP_STRUCT__entry(
>>                 __field(const void *, dr)
>>                 __field(u32, xid)
>> -               __string(addr, dr->xprt->xpt_remotebuf)
>> +               __dynamic_array(u8, addr, dr->addrlen)
>>         ),
>> =20
>>         TP_fast_assign(
>>                 __entry->dr =3D dr;
>>                 __entry->xid =3D be32_to_cpu(*(__be32 *)(dr->args +
>>                                                        (dr-
>>> xprt_hlen>>2)));
>> -               __assign_str(addr, dr->xprt->xpt_remotebuf);
>> +               memcpy(__get_dynamic_array(addr), &dr->addr, dr-
>>> addrlen);
>>         ),
>> =20
>> -       TP_printk("addr=3D%s dr=3D%p xid=3D0x%08x", __get_str(addr),
>> __entry->dr,
>> -               __entry->xid)
>> +       TP_printk("addr=3D%pISpc dr=3D%p xid=3D0x%08x",
>> +               (struct sockaddr *)__get_dynamic_array(addr),
>> +               __entry->dr, __entry->xid)
>>  );
>>=20
>=20
> Have you tested this? I found myself hitting the warning
>=20
> "event %s has unsafe dereference of argument %d"
>=20
> in test_event_printk() when I tried to use something similar for the
> NFS code.

The warning is fixed by c6ced22997ad ("tracing: Update print
fmt check to handle new __get_sockaddr() macro"). I think
this means that along with the above patch, c6ced22997ad will
need to be backported to some recent stable kernels.

If you're adding dynamically-sized sockaddr fields in trace
records, I invite you to consider __sockaddr/__get_sockaddr(),
added in v5.18.


--
Chuck Lever