| Message ID | 20201123153447.2e24c678@gandalf.local.home (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tracefs utils: Do not free a the buffer on a zero size str_read_file() | expand |
On Mon, Nov 23, 2020 at 10:35 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org> > > If a file has no size (nothing is read), then str_read_file() frees the > buffer and returns zero. The problem is that all callers of str_read_file() > uses the buffer supplied if the value returned is not a negative. This > causes the freed buffer being used by the callers if the file read existed > but had no content. > > This is apparent when using a copy of the tracefs directory, where some file > exist, but have no content, then loading the events would cause a segfault. > > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > --- > tracefs-utils.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tracefs-utils.c b/tracefs-utils.c > index 326b455..690506c 100644 > --- a/tracefs-utils.c > +++ b/tracefs-utils.c > @@ -220,7 +220,7 @@ __hidden int str_read_file(const char *file, char **buffer) > } while (r > 0); > > close(fd); > - if (r == 0 && size > 0) { > + if (r == 0) { > buf[size] = '\0'; If size is 0, in case of an empty file, then buf should also be NULL and this assignment will be on invalid memory. > *buffer = buf; > } else > -- > 2.25.4 >
On Tue, 24 Nov 2020 07:44:14 +0200 Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote: > > index 326b455..690506c 100644 > > --- a/tracefs-utils.c > > +++ b/tracefs-utils.c > > @@ -220,7 +220,7 @@ __hidden int str_read_file(const char *file, char **buffer) > > } while (r > 0); > > > > close(fd); > > - if (r == 0 && size > 0) { > > + if (r == 0) { > > buf[size] = '\0'; > > If size is 0, in case of an empty file, then buf should also be NULL > and this assignment > will be on invalid memory. I quickly realized that, and sent out a v2 ;-) https://lore.kernel.org/r/20201123154607.5e43e1ff@gandalf.local.home -- Steve > > > *buffer = buf; > > } else > > --
diff --git a/tracefs-utils.c b/tracefs-utils.c index 326b455..690506c 100644 --- a/tracefs-utils.c +++ b/tracefs-utils.c @@ -220,7 +220,7 @@ __hidden int str_read_file(const char *file, char **buffer) } while (r > 0); close(fd); - if (r == 0 && size > 0) { + if (r == 0) { buf[size] = '\0'; *buffer = buf; } else