* [PATCH] perf probe: check for dup and fdopen failures
@ 2016-08-12 21:44 Colin King
2016-08-12 21:55 ` Arnaldo Carvalho de Melo
2016-08-16 18:16 ` [tip:perf/urgent] perf probe: Check " tip-bot for Colin Ian King
0 siblings, 2 replies; 4+ messages in thread
From: Colin King @ 2016-08-12 21:44 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Masami Hiramatsu, Namhyung Kim, Jiri Olsa,
Wang Nan
Cc: linux-kernel
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.
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)
+ return NULL;
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] perf probe: check for dup and fdopen failures
2016-08-12 21:44 [PATCH] perf probe: check for dup and fdopen failures Colin King
@ 2016-08-12 21:55 ` Arnaldo Carvalho de Melo
2016-08-12 23:55 ` Masami Hiramatsu
2016-08-16 18:16 ` [tip:perf/urgent] perf probe: Check " tip-bot for Colin Ian King
1 sibling, 1 reply; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-08-12 21:55 UTC (permalink / raw)
To: Colin King
Cc: Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Masami Hiramatsu,
Namhyung Kim, Jiri Olsa, Wang Nan, linux-kernel
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.
>
> 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?
- 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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] perf probe: check for dup and fdopen failures
2016-08-12 21:55 ` Arnaldo Carvalho de Melo
@ 2016-08-12 23:55 ` Masami Hiramatsu
0 siblings, 0 replies; 4+ messages in thread
From: Masami Hiramatsu @ 2016-08-12 23:55 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Colin King, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
Masami Hiramatsu, Namhyung Kim, Jiri Olsa, Wang Nan, linux-kernel
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>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [tip:perf/urgent] perf probe: Check for dup and fdopen failures
2016-08-12 21:44 [PATCH] perf probe: check for dup and fdopen failures Colin King
2016-08-12 21:55 ` Arnaldo Carvalho de Melo
@ 2016-08-16 18:16 ` tip-bot for Colin Ian King
1 sibling, 0 replies; 4+ messages in thread
From: tip-bot for Colin Ian King @ 2016-08-16 18:16 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, peterz, linux-kernel, namhyung, jolsa, tglx, mingo, hpa,
alexander.shishkin, mhiramat, colin.king, wangnan0
Commit-ID: 0325862dc364d8af524bf2db53ef4360ed55b989
Gitweb: http://git.kernel.org/tip/0325862dc364d8af524bf2db53ef4360ed55b989
Author: Colin Ian King <colin.king@canonical.com>
AuthorDate: Fri, 12 Aug 2016 22:44:56 +0100
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 15 Aug 2016 17:06:19 -0300
perf probe: Check for dup and fdopen failures
dup and fdopen can potentially fail, so add some extra
error handling checks rather than assuming they always work.
Signed-off-by: Colin King <colin.king@canonical.com>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1471038296-12956-1-git-send-email-colin.king@canonical.com
[ Free resources when those functions (now being verified) fail ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/probe-file.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 9aed9c3..a8e7623 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,14 @@ struct strlist *probe_file__get_rawlist(int fd)
sl = strlist__new(NULL, NULL);
- fp = fdopen(dup(fd), "r");
+ fddup = dup(fd);
+ if (fddup < 0)
+ goto out_free_sl;
+
+ fp = fdopen(fddup, "r");
+ if (!fp)
+ goto out_close_fddup;
+
while (!feof(fp)) {
p = fgets(buf, MAX_CMDLEN, fp);
if (!p)
@@ -163,6 +170,12 @@ struct strlist *probe_file__get_rawlist(int fd)
fclose(fp);
return sl;
+
+out_close_fddup:
+ close(fddup);
+out_free_sl:
+ strlist__delete(sl);
+ return NULL;
}
static struct strlist *__probe_file__get_namelist(int fd, bool include_group)
@@ -447,10 +460,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;
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-08-16 18:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-12 21:44 [PATCH] perf probe: check for dup and fdopen failures Colin King
2016-08-12 21:55 ` Arnaldo Carvalho de Melo
2016-08-12 23:55 ` Masami Hiramatsu
2016-08-16 18:16 ` [tip:perf/urgent] perf probe: Check " tip-bot for Colin Ian King
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.