Message ID | 164182978641.8391.8277203495236105391.stgit@bazille.1015granger.net (mailing list archive) |
---|---|
Headers | show |
Series | Fix sockaddr handling in NFSD trace points | expand |
On Mon, 10 Jan 2022 10:56:49 -0500 Chuck Lever <chuck.lever@oracle.com> wrote: > The patches in this series address a simple buffer over-read bug in > the Linux NFS server. > > However I was thinking it would be nice to have trace helpers to > deal safely with generic socket addresses. I'd like to be able to > treat them the same way we currently treat strings. So for example: > > > #define field_sockaddr(field, len) __dynamic_array(u8, field, len) > #define assign_sockaddr(dest, src, len) memcpy(__get_dynamic_array(dest), src, len) > #define __get_sockaddr(field) ((struct sockaddr *)__get_dynamic_array(field)) > > TRACE_EVENT(sockaddr_example, > TP_PROTO( > const struct sockaddr *sap, > size_t salen > ), > TP_ARGS(sap, salen), > TP_STRUCT__entry( > __field_sockaddr(addr, salen) > ), > TP_fast_assign( > __assign_sockaddr(addr, sap, salen); > ), > TP_printk("addr=%pIS", __get_sockaddr(addr)) > ); > > > should be able to store any address family in a dynamically-sized > array field (addr). > > I haven't quite been able to figure out how to handle the > TP_printk() part of this equation. `trace-cmd report` displays > something like "addr=ARG TYPE NOT FIELD BUT 7". > > Thoughts or advice appreciated. I'll take a look into it. Thanks, -- Steve
On Mon, 10 Jan 2022 11:05:35 -0500 Steven Rostedt <rostedt@goodmis.org> wrote: > > I haven't quite been able to figure out how to handle the > > TP_printk() part of this equation. `trace-cmd report` displays > > something like "addr=ARG TYPE NOT FIELD BUT 7". > > > > Thoughts or advice appreciated. > > I'll take a look into it. If you add this to libtraceevent, it will work: diff --git a/src/event-parse.c b/src/event-parse.c index 9bd605d..033529c 100644 --- a/src/event-parse.c +++ b/src/event-parse.c @@ -5127,6 +5127,8 @@ static int print_ipsa_arg(struct trace_seq *s, const char *ptr, char i, unsigned char *buf; struct sockaddr_storage *sa; bool reverse = false; + unsigned int offset; + unsigned int len; int rc = 0; int ret; @@ -5152,27 +5154,42 @@ static int print_ipsa_arg(struct trace_seq *s, const char *ptr, char i, return rc; } - if (arg->type != TEP_PRINT_FIELD) { - trace_seq_printf(s, "ARG TYPE NOT FIELD BUT %d", arg->type); - return rc; - } + /* evaluate if the arg has a typecast */ + while (arg->type == TEP_PRINT_TYPE) + arg = arg->typecast.item; + + if (arg->type == TEP_PRINT_FIELD) { - if (!arg->field.field) { - arg->field.field = - tep_find_any_field(event, arg->field.name); if (!arg->field.field) { - do_warning("%s: field %s not found", - __func__, arg->field.name); - return rc; + arg->field.field = + tep_find_any_field(event, arg->field.name); + if (!arg->field.field) { + do_warning("%s: field %s not found", + __func__, arg->field.name); + return rc; + } } + + offset = arg->field.field->offset; + len = arg->field.field->size; + + } else if (arg->type == TEP_PRINT_DYNAMIC_ARRAY) { + + dynamic_offset_field(event->tep, arg->dynarray.field, data, + size, &offset, &len); + + } else { + trace_seq_printf(s, "ARG NOT FIELD NOR DYNAMIC ARRAY BUT TYPE %d", + arg->type); + return rc; } - sa = (struct sockaddr_storage *) (data + arg->field.field->offset); + sa = (struct sockaddr_storage *)(data + offset); if (sa->ss_family == AF_INET) { struct sockaddr_in *sa4 = (struct sockaddr_in *) sa; - if (arg->field.field->size < sizeof(struct sockaddr_in)) { + if (len < sizeof(struct sockaddr_in)) { trace_seq_printf(s, "INVALIDIPv4"); return rc; } @@ -5185,7 +5202,7 @@ static int print_ipsa_arg(struct trace_seq *s, const char *ptr, char i, } else if (sa->ss_family == AF_INET6) { struct sockaddr_in6 *sa6 = (struct sockaddr_in6 *) sa; - if (arg->field.field->size < sizeof(struct sockaddr_in6)) { + if (len < sizeof(struct sockaddr_in6)) { trace_seq_printf(s, "INVALIDIPv6"); return rc; }
> On Jan 10, 2022, at 6:31 PM, Steven Rostedt <rostedt@goodmis.org> wrote: > > On Mon, 10 Jan 2022 11:05:35 -0500 > Steven Rostedt <rostedt@goodmis.org> wrote: > >>> I haven't quite been able to figure out how to handle the >>> TP_printk() part of this equation. `trace-cmd report` displays >>> something like "addr=ARG TYPE NOT FIELD BUT 7". >>> >>> Thoughts or advice appreciated. >> >> I'll take a look into it. > > If you add this to libtraceevent, it will work: Thank you Steven! I will set up my current test system with a locally-built trace-cmd and try this out. I can send a proper patch that introduces the helper macros in my cover letter's pseudo-code example today or tomorrow. > diff --git a/src/event-parse.c b/src/event-parse.c > index 9bd605d..033529c 100644 > --- a/src/event-parse.c > +++ b/src/event-parse.c > @@ -5127,6 +5127,8 @@ static int print_ipsa_arg(struct trace_seq *s, const char *ptr, char i, > unsigned char *buf; > struct sockaddr_storage *sa; > bool reverse = false; > + unsigned int offset; > + unsigned int len; > int rc = 0; > int ret; > > @@ -5152,27 +5154,42 @@ static int print_ipsa_arg(struct trace_seq *s, const char *ptr, char i, > return rc; > } > > - if (arg->type != TEP_PRINT_FIELD) { > - trace_seq_printf(s, "ARG TYPE NOT FIELD BUT %d", arg->type); > - return rc; > - } > + /* evaluate if the arg has a typecast */ > + while (arg->type == TEP_PRINT_TYPE) > + arg = arg->typecast.item; > + > + if (arg->type == TEP_PRINT_FIELD) { > > - if (!arg->field.field) { > - arg->field.field = > - tep_find_any_field(event, arg->field.name); > if (!arg->field.field) { > - do_warning("%s: field %s not found", > - __func__, arg->field.name); > - return rc; > + arg->field.field = > + tep_find_any_field(event, arg->field.name); > + if (!arg->field.field) { > + do_warning("%s: field %s not found", > + __func__, arg->field.name); > + return rc; > + } > } > + > + offset = arg->field.field->offset; > + len = arg->field.field->size; > + > + } else if (arg->type == TEP_PRINT_DYNAMIC_ARRAY) { > + > + dynamic_offset_field(event->tep, arg->dynarray.field, data, > + size, &offset, &len); > + > + } else { > + trace_seq_printf(s, "ARG NOT FIELD NOR DYNAMIC ARRAY BUT TYPE %d", > + arg->type); > + return rc; > } > > - sa = (struct sockaddr_storage *) (data + arg->field.field->offset); > + sa = (struct sockaddr_storage *)(data + offset); > > if (sa->ss_family == AF_INET) { > struct sockaddr_in *sa4 = (struct sockaddr_in *) sa; > > - if (arg->field.field->size < sizeof(struct sockaddr_in)) { > + if (len < sizeof(struct sockaddr_in)) { > trace_seq_printf(s, "INVALIDIPv4"); > return rc; > } > @@ -5185,7 +5202,7 @@ static int print_ipsa_arg(struct trace_seq *s, const char *ptr, char i, > } else if (sa->ss_family == AF_INET6) { > struct sockaddr_in6 *sa6 = (struct sockaddr_in6 *) sa; > > - if (arg->field.field->size < sizeof(struct sockaddr_in6)) { > + if (len < sizeof(struct sockaddr_in6)) { > trace_seq_printf(s, "INVALIDIPv6"); > return rc; > } -- Chuck Lever