* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox