> On Apr 6, 2022, at 4:34 PM, Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> wrote:
>
> 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.
>>
>> 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.
>>
>> Fixes: ece200ddd54b ("sunrpc: Save remote presentation address in
>> svc_xprt for trace events")
>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>> ---
>> include/trace/events/sunrpc.h | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> 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)
>> ),
>>
>> TP_fast_assign(
>> __entry->dr = dr;
>> __entry->xid = 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);
>> ),
>>
>> - TP_printk("addr=%s dr=%p xid=0x%08x", __get_str(addr),
>> __entry->dr,
>> - __entry->xid)
>> + TP_printk("addr=%pISpc dr=%p xid=0x%08x",
>> + (struct sockaddr *)__get_dynamic_array(addr),
>> + __entry->dr, __entry->xid)
>> );
>>
>
> Have you tested this? I found myself hitting the warning
>
> "event %s has unsafe dereference of argument %d"
>
> 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
