Messages in this thread | | | Date | Sat, 13 Aug 2016 08:55:54 +0900 | From | Masami Hiramatsu <> | Subject | Re: [PATCH] perf probe: check for dup and fdopen failures |
| |
On Fri, 12 Aug 2016 18:55:17 -0300 Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> Em Fri, Aug 12, 2016 at 10:44:56PM +0100, Colin King escreveu: > > From: Colin Ian King <colin.king@canonical.com> > > > > dup and fdopen can potentially fail, so add some extra > > error handling checks rather than assuming they always work.
Thanks, I forgot to handle that.
> > > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > > --- > > tools/perf/util/probe-file.c | 16 ++++++++++++---- > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c > > index 9aed9c3..f3df939 100644 > > --- a/tools/perf/util/probe-file.c > > +++ b/tools/perf/util/probe-file.c > > @@ -133,7 +133,7 @@ int probe_file__open_both(int *kfd, int *ufd, int flag) > > /* Get raw string list of current kprobe_events or uprobe_events */ > > struct strlist *probe_file__get_rawlist(int fd) > > { > > - int ret, idx; > > + int ret, idx, fddup; > > FILE *fp; > > char buf[MAX_CMDLEN]; > > char *p; > > @@ -144,7 +144,12 @@ struct strlist *probe_file__get_rawlist(int fd) > > > > sl = strlist__new(NULL, NULL); > > > > - fp = fdopen(dup(fd), "r"); > > + fddup = dup(fd); > > + if (fddup < 0) > > + return NULL; > > + fp = fdopen(fddup, "r"); > > + if (!fp) > > here we leak fddup, no? > > > + return NULL; > > I.e. this should be: > > if (!fp) { > close(fddup); > return NULL; > } > > Ack?
Correct, it should be closed. So both fixes look good to me.
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Thanks!
> > - Arnaldo > > > while (!feof(fp)) { > > p = fgets(buf, MAX_CMDLEN, fp); > > if (!p) > > @@ -447,10 +452,13 @@ static int probe_cache__load(struct probe_cache *pcache) > > { > > struct probe_cache_entry *entry = NULL; > > char buf[MAX_CMDLEN], *p; > > - int ret = 0; > > + int ret = 0, fddup; > > FILE *fp; > > > > - fp = fdopen(dup(pcache->fd), "r"); > > + fddup = dup(pcache->fd); > > + if (fddup < 0) > > + return -errno; > > + fp = fdopen(fddup, "r"); > > if (!fp) > > return -EINVAL; > > > > -- > > 2.8.1
-- Masami Hiramatsu <mhiramat@kernel.org>
| |