> On Jun 25, 2021, at 2:57 PM, Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> wrote: > > On Fri, 2021-06-25 at 18:05 +0000, Chuck Lever III wrote: >> >> >>> On Jun 25, 2021, at 12:28 PM, Trond Myklebust >>> <trondmy@xxxxxxxxxxxxxxx> wrote: >>> >>> On Fri, 2021-06-25 at 11:12 -0400, Chuck Lever wrote: >>>> The double copy of the string is a mistake, plus __assign_str() >>>> uses strlen(), which is wrong to do on a string that isn't >>>> guaranteed to be NUL-terminated. >>>> >>>> Fixes: 6019ce0742ca ("NFSD: Add a tracepoint to record directory >>>> entry encoding") >>>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx> >>>> --- >>>> fs/nfsd/trace.h | 1 - >>>> 1 file changed, 1 deletion(-) >>>> >>>> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h >>>> index 27a93ebd1d80..89dccced526a 100644 >>>> --- a/fs/nfsd/trace.h >>>> +++ b/fs/nfsd/trace.h >>>> @@ -408,7 +408,6 @@ TRACE_EVENT(nfsd_dirent, >>>> __entry->ino = ino; >>>> __entry->len = namlen; >>>> memcpy(__get_str(name), name, namlen); >>>> - __assign_str(name, name); >>>> ), >>>> TP_printk("fh_hash=0x%08x ino=%llu name=%.*s", >>>> __entry->fh_hash, __entry->ino, >>>> >>>> >>> >>> Why not just store it as a NUL terminated string and save a few bytes >>> by getting rid of the integer-sized storage of the length? >> >> Stephen is adding some helpers to do that in the next merge >> window. For now, I need to fix this oops, and this is the >> fastest way to do that. > > > Won't this suffice? Yes of course it will. As I said, Stephen has some trace macros lines up for the next merge window that I would prefer to use. I will be posting patches that use those as soon as 5.14-rc1 appears. For now, a single line fix is needed to prevent an oops in 5.13. > 8<--------------------------------------- > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h > index 27a93ebd1d80..799168774ccf 100644 > --- a/fs/nfsd/trace.h > +++ b/fs/nfsd/trace.h > @@ -400,19 +400,17 @@ TRACE_EVENT(nfsd_dirent, > TP_STRUCT__entry( > __field(u32, fh_hash) > __field(u64, ino) > - __field(int, len) > - __dynamic_array(unsigned char, name, namlen) > + __dynamic_array(unsigned char, name, namlen + 1) > ), > TP_fast_assign( > __entry->fh_hash = fhp ? knfsd_fh_hash(&fhp->fh_handle) : 0; > __entry->ino = ino; > - __entry->len = namlen; > memcpy(__get_str(name), name, namlen); > - __assign_str(name, name); > + __get_str(name)[namlen] = 0; > ), > - TP_printk("fh_hash=0x%08x ino=%llu name=%.*s", > + TP_printk("fh_hash=0x%08x ino=%llu name=%s", > __entry->fh_hash, __entry->ino, > - __entry->len, __get_str(name)) > + __get_str(name)) > ) > > #include "state.h" > > > -- > Trond Myklebust > Linux NFS client maintainer, Hammerspace > trond.myklebust@xxxxxxxxxxxxxxx -- Chuck Lever