Re: [PATCH] 9p: prevent read overrun in protocol dump tracepoint

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 02 Dec 2023 14:05:24 +0100
Christian Schoenebeck <linux_oss@xxxxxxxxxxxxx> wrote:

> > > --- a/include/trace/events/9p.h
> > > +++ b/include/trace/events/9p.h
> > > @@ -185,7 +185,8 @@ TRACE_EVENT(9p_protocol_dump,
> > >  		    __entry->clnt   =  clnt;
> > >  		    __entry->type   =  pdu->id;
> > >  		    __entry->tag    =  pdu->tag;
> > > -		    memcpy(__entry->line, pdu->sdata, P9_PROTO_DUMP_SZ);
> > > +		    memcpy(__entry->line, pdu->sdata,
> > > +				min(pdu->capacity, P9_PROTO_DUMP_SZ));
> > >  		    ),
> > >  	    TP_printk("clnt %lu %s(tag = %d)\n%.3x: %16ph\n%.3x: %16ph\n",
> > >  		      (unsigned long)__entry->clnt, show_9p_op(__entry->type),  
> 
> AFAICS __entry is a local variable on stack, and array __entry->line not
> intialized with zeros, i.e. the dump would contain trash at the end. Maybe
> prepending memset() before memcpy()?

__entry is a macro that points into the ring buffer that gets allocated
before this is called. TRACE_EVENT() has a __dynamic_array() field that
can handle variable length arrays. What you can do is turn this into
something like:

TRACE_EVENT(9p_protocol_dump,
            TP_PROTO(struct p9_client *clnt, struct p9_fcall *pdu),

            TP_ARGS(clnt, pdu),

            TP_STRUCT__entry(
                    __field(    void *,         clnt                            )
                    __field(    __u8,           type                            )
                    __field(    __u16,          tag                             )
                    __dynamic_array(unsigned char,  line, min(pdu->capacity, P9_PROTO_DUMP_SZ) )
                    ),

            TP_fast_assign(
                    __entry->clnt   =  clnt;
                    __entry->type   =  pdu->id;
                    __entry->tag    =  pdu->tag;
                    memcpy(__get_dynamic_array(line), pdu->sdata,
			   min(pdu->capacity, P9_PROTO_DUMP_SZ));
                    ),
            TP_printk("clnt %lu %s(tag = %d)\n%.3x: %16ph\n%.3x: %16ph\n",
                      (unsigned long)__entry->clnt, show_9p_op(__entry->type),
                      __entry->tag, 0, __get_dynamic_array(line), 16,
		      __get_dynamic_array(line) + 16)
 );

-- Steve




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux