| Message ID | 20221215183151.2685460-1-irogers@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1,1/2] libtracefs: Move initialization below a null test. | expand |
On Thu, 15 Dec 2022 10:31:50 -0800 Ian Rogers <irogers@google.com> wrote: > Computing the address from a NULL pointer results in undefined behavior > which things like undefined behavior sanitizer promote into real > failures. I thought commit aff006d4af0c7 ("libtracefs: Do not initialize with NULL offsets") was suppose to fix this. Oh, I think I may have forgotten to remove the check against dynevent! Does this work instead? -- Steve diff --git a/src/tracefs-dynevents.c b/src/tracefs-dynevents.c index 48bb26a96c58..7a3c45ce25a3 100644 --- a/src/tracefs-dynevents.c +++ b/src/tracefs-dynevents.c @@ -713,9 +713,6 @@ dynevent_info(struct tracefs_dynevent *dynevent, char **system, &dynevent->address, &dynevent->format }; int i; - if (!dynevent) - return TRACEFS_DYNEVENT_UNKNOWN; - for (i = 0; i < ARRAY_SIZE(lv); i++) { if (lv[i]) { if (*rv[i]) {
On Thu, Dec 15, 2022 at 11:02 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Thu, 15 Dec 2022 10:31:50 -0800 > Ian Rogers <irogers@google.com> wrote: > > > Computing the address from a NULL pointer results in undefined behavior > > which things like undefined behavior sanitizer promote into real > > failures. > > I thought commit aff006d4af0c7 ("libtracefs: Do not initialize with NULL > offsets") was suppose to fix this. It may have done, sorry for not keeping track. I went to my libtracefs git repo to fix the patch 2 issue and saw that this change didn't vanish with a rebase. > Oh, I think I may have forgotten to remove the check against dynevent! > > Does this work instead? > > -- Steve > > diff --git a/src/tracefs-dynevents.c b/src/tracefs-dynevents.c > index 48bb26a96c58..7a3c45ce25a3 100644 > --- a/src/tracefs-dynevents.c > +++ b/src/tracefs-dynevents.c > @@ -713,9 +713,6 @@ dynevent_info(struct tracefs_dynevent *dynevent, char **system, > &dynevent->address, &dynevent->format }; > int i; > > - if (!dynevent) > - return TRACEFS_DYNEVENT_UNKNOWN; > - I think the issue is above in &dynevent->... that dynevent may be NULL. It seems from removing this that dynevent can't be NULL and so the undefined behavior can never happen - in which case there's no issue to fix and the test is unnecessary and should be removed. That's a long winded way of saying I think that's a correct patch and this patch is unnecessary :-) Thanks, Ian > for (i = 0; i < ARRAY_SIZE(lv); i++) { > if (lv[i]) { > if (*rv[i]) {
diff --git a/src/tracefs-dynevents.c b/src/tracefs-dynevents.c index 48bb26a..02e8023 100644 --- a/src/tracefs-dynevents.c +++ b/src/tracefs-dynevents.c @@ -709,13 +709,18 @@ dynevent_info(struct tracefs_dynevent *dynevent, char **system, char **event, char **prefix, char **addr, char **format) { char **lv[] = { system, event, prefix, addr, format }; - char **rv[] = { &dynevent->system, &dynevent->event, &dynevent->prefix, - &dynevent->address, &dynevent->format }; + char **rv[] = { NULL, NULL, NULL, NULL, NULL }; int i; if (!dynevent) return TRACEFS_DYNEVENT_UNKNOWN; + rv[0] = &dynevent->system; + rv[1] = &dynevent->event; + rv[2] = &dynevent->prefix; + rv[3] = &dynevent->address; + rv[4] = &dynevent->format; + for (i = 0; i < ARRAY_SIZE(lv); i++) { if (lv[i]) { if (*rv[i]) {
Computing the address from a NULL pointer results in undefined behavior which things like undefined behavior sanitizer promote into real failures. Signed-off-by: Ian Rogers <irogers@google.com> --- src/tracefs-dynevents.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)