* [PATCH 0/7] ethtool: misc fixes for Coverity issues
@ 2016-09-30 19:56 John W. Linville
2016-09-30 19:56 ` [PATCH 1/7] ethtool: avoid NULL pointer dereference in do_permaddr John W. Linville
` (7 more replies)
0 siblings, 8 replies; 12+ messages in thread
From: John W. Linville @ 2016-09-30 19:56 UTC (permalink / raw)
To: netdev; +Cc: John W. Linville
I requested a Coverity scan of the current ethtool sources. There
weren't many issues identified, but it was non-zero.
Most/all of the resource leak issues can be explained away as
unimportant due to the fact that ethtool invocations are short-term
processes. Nevertheless, I prefer to have those issues "fixed" both
to avoid them as distractions and just to promote better coding
practices overall.
These are "fresh off the press" -- I'll re-review them again before
applying and pushing them. I appreciate any extra eyeballs that can
review them as well.
John
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/7] ethtool: avoid NULL pointer dereference in do_permaddr
2016-09-30 19:56 [PATCH 0/7] ethtool: misc fixes for Coverity issues John W. Linville
@ 2016-09-30 19:56 ` John W. Linville
2016-09-30 19:56 ` [PATCH 2/7] ethtool: avoid resource leak of strings in do_gprivflags John W. Linville
` (6 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: John W. Linville @ 2016-09-30 19:56 UTC (permalink / raw)
To: netdev; +Cc: John W. Linville
Coverity issue: 1363118
Fixes: fef1c4a19703 ("ethtool: add get permanent address option")
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
ethtool.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/ethtool.c b/ethtool.c
index 6287b559c699..aa3ef5ed2f75 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -3941,6 +3941,11 @@ static int do_permaddr(struct cmd_context *ctx)
struct ethtool_perm_addr *epaddr;
epaddr = malloc(sizeof(struct ethtool_perm_addr) + MAX_ADDR_LEN);
+ if (!epaddr) {
+ perror("Cannot allocate memory for operation");
+ return 1;
+ }
+
epaddr->cmd = ETHTOOL_GPERMADDR;
epaddr->size = MAX_ADDR_LEN;
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/7] ethtool: avoid resource leak of strings in do_gprivflags
2016-09-30 19:56 [PATCH 0/7] ethtool: misc fixes for Coverity issues John W. Linville
2016-09-30 19:56 ` [PATCH 1/7] ethtool: avoid NULL pointer dereference in do_permaddr John W. Linville
@ 2016-09-30 19:56 ` John W. Linville
2016-10-03 17:54 ` Jarod Wilson
2016-10-04 14:05 ` [PATCH v2 " John W. Linville
2016-09-30 19:56 ` [PATCH 3/7] ethtool: plug resource leaks of defs and features in do_gfeatures John W. Linville
` (5 subsequent siblings)
7 siblings, 2 replies; 12+ messages in thread
From: John W. Linville @ 2016-09-30 19:56 UTC (permalink / raw)
To: netdev; +Cc: John W. Linville
Coverity issue: 1363119
Fixes: e1ee596326ae ("Add support for querying and setting private flags")
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
ethtool.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/ethtool.c b/ethtool.c
index aa3ef5ed2f75..0885a61097ad 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4205,7 +4205,7 @@ static int do_gprivflags(struct cmd_context *ctx)
struct ethtool_gstrings *strings;
struct ethtool_value flags;
unsigned int i;
- int max_len = 0, cur_len;
+ int max_len = 0, cur_len, rc;
if (ctx->argc != 0)
exit_bad_args();
@@ -4215,11 +4215,13 @@ static int do_gprivflags(struct cmd_context *ctx)
1);
if (!strings) {
perror("Cannot get private flag names");
- return 1;
+ rc = 1;
+ goto err;
}
if (strings->len == 0) {
fprintf(stderr, "No private flags defined\n");
- return 1;
+ rc = 1;
+ goto err;
}
if (strings->len > 32) {
/* ETHTOOL_GPFLAGS can only cover 32 flags */
@@ -4230,7 +4232,8 @@ static int do_gprivflags(struct cmd_context *ctx)
flags.cmd = ETHTOOL_GPFLAGS;
if (send_ioctl(ctx, &flags)) {
perror("Cannot get private flags");
- return 1;
+ rc = 1;
+ goto err;
}
/* Find longest string and align all strings accordingly */
@@ -4248,7 +4251,12 @@ static int do_gprivflags(struct cmd_context *ctx)
(const char *)strings->data + i * ETH_GSTRING_LEN,
(flags.data & (1U << i)) ? "on" : "off");
- return 0;
+ rc = 0;
+
+err:
+ if (strings)
+ free(strings);
+ return rc;
}
static int do_sprivflags(struct cmd_context *ctx)
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/7] ethtool: plug resource leaks of defs and features in do_gfeatures
2016-09-30 19:56 [PATCH 0/7] ethtool: misc fixes for Coverity issues John W. Linville
2016-09-30 19:56 ` [PATCH 1/7] ethtool: avoid NULL pointer dereference in do_permaddr John W. Linville
2016-09-30 19:56 ` [PATCH 2/7] ethtool: avoid resource leak of strings in do_gprivflags John W. Linville
@ 2016-09-30 19:56 ` John W. Linville
2016-09-30 19:56 ` [PATCH 4/7] ethtool: fix leakage of defs resources in do_sfeatures John W. Linville
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: John W. Linville @ 2016-09-30 19:56 UTC (permalink / raw)
To: netdev; +Cc: John W. Linville
Coverity issues: 1363120, 1363121
Fixes: 6042804cf6ec ("Change -k/-K options to use ETHTOOL_{G,S}FEATURES")
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
ethtool.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/ethtool.c b/ethtool.c
index 0885a61097ad..09486615a5bb 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -2233,10 +2233,13 @@ static int do_gfeatures(struct cmd_context *ctx)
features = get_features(ctx, defs);
if (!features) {
fprintf(stdout, "no feature info available\n");
+ free(defs);
return 1;
}
dump_features(defs, features, NULL);
+ free(features);
+ free(defs);
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/7] ethtool: fix leakage of defs resources in do_sfeatures
2016-09-30 19:56 [PATCH 0/7] ethtool: misc fixes for Coverity issues John W. Linville
` (2 preceding siblings ...)
2016-09-30 19:56 ` [PATCH 3/7] ethtool: plug resource leaks of defs and features in do_gfeatures John W. Linville
@ 2016-09-30 19:56 ` John W. Linville
2016-09-30 19:56 ` [PATCH 5/7] ethtool: fix leakage of efeatures " John W. Linville
` (3 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: John W. Linville @ 2016-09-30 19:56 UTC (permalink / raw)
To: netdev; +Cc: John W. Linville
Coverity issue: 1363123
Fixes: 6042804cf6ecc ("Change -k/-K options to use ETHTOOL_{G,S}FEATURES")
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
ethtool.c | 41 ++++++++++++++++++++++++++++-------------
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/ethtool.c b/ethtool.c
index 09486615a5bb..2fd3393ca5f6 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -2253,7 +2253,7 @@ static int do_sfeatures(struct cmd_context *ctx)
struct cmdline_info *cmdline_features;
struct feature_state *old_state, *new_state;
struct ethtool_value eval;
- int err;
+ int err, rc;
int i, j;
defs = get_feature_defs(ctx);
@@ -2267,7 +2267,8 @@ static int do_sfeatures(struct cmd_context *ctx)
sizeof(efeatures->features[0]));
if (!efeatures) {
perror("Cannot parse arguments");
- return 1;
+ rc = 1;
+ goto err;
}
efeatures->cmd = ETHTOOL_SFEATURES;
efeatures->size = FEATURE_BITS_TO_BLOCKS(defs->n_features);
@@ -2285,7 +2286,8 @@ static int do_sfeatures(struct cmd_context *ctx)
sizeof(cmdline_features[0]));
if (!cmdline_features) {
perror("Cannot parse arguments");
- return 1;
+ rc = 1;
+ goto err;
}
for (i = 0; i < ARRAY_SIZE(off_flag_def); i++)
flag_to_cmdline_info(off_flag_def[i].short_name,
@@ -2304,12 +2306,15 @@ static int do_sfeatures(struct cmd_context *ctx)
if (!any_changed) {
fprintf(stdout, "no features changed\n");
- return 0;
+ rc = 0;
+ goto err;
}
old_state = get_features(ctx, defs);
- if (!old_state)
- return 1;
+ if (!old_state) {
+ rc = 1;
+ goto err;
+ }
if (efeatures) {
/* For each offload that the user specified, update any
@@ -2353,7 +2358,8 @@ static int do_sfeatures(struct cmd_context *ctx)
err = send_ioctl(ctx, efeatures);
if (err < 0) {
perror("Cannot set device feature settings");
- return 1;
+ rc = 1;
+ goto err;
}
} else {
for (i = 0; i < ARRAY_SIZE(off_flag_def); i++) {
@@ -2368,7 +2374,8 @@ static int do_sfeatures(struct cmd_context *ctx)
fprintf(stderr,
"Cannot set device %s settings: %m\n",
off_flag_def[i].long_name);
- return 1;
+ rc = 1;
+ goto err;
}
}
}
@@ -2382,15 +2389,18 @@ static int do_sfeatures(struct cmd_context *ctx)
err = send_ioctl(ctx, &eval);
if (err) {
perror("Cannot set device flag settings");
- return 92;
+ rc = 92;
+ goto err;
}
}
}
/* Compare new state with requested state */
new_state = get_features(ctx, defs);
- if (!new_state)
- return 1;
+ if (!new_state) {
+ rc = 1;
+ goto err;
+ }
any_changed = new_state->off_flags != old_state->off_flags;
any_mismatch = (new_state->off_flags !=
((old_state->off_flags & ~off_flags_mask) |
@@ -2409,13 +2419,18 @@ static int do_sfeatures(struct cmd_context *ctx)
if (!any_changed) {
fprintf(stderr,
"Could not change any device features\n");
- return 1;
+ rc = 1;
+ goto err;
}
printf("Actual changes:\n");
dump_features(defs, new_state, old_state);
}
- return 0;
+ rc = 0;
+
+err:
+ free(defs);
+ return rc;
}
static struct ethtool_link_usettings *
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 5/7] ethtool: fix leakage of efeatures resources in do_sfeatures
2016-09-30 19:56 [PATCH 0/7] ethtool: misc fixes for Coverity issues John W. Linville
` (3 preceding siblings ...)
2016-09-30 19:56 ` [PATCH 4/7] ethtool: fix leakage of defs resources in do_sfeatures John W. Linville
@ 2016-09-30 19:56 ` John W. Linville
2016-09-30 19:56 ` [PATCH 6/7] ethtool: fix leakage of strings resources in do_sprivflags John W. Linville
` (2 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: John W. Linville @ 2016-09-30 19:56 UTC (permalink / raw)
To: netdev; +Cc: John W. Linville
Coverity issue: 1363122
Fixes: 6042804cf6ecc ("Change -k/-K options to use ETHTOOL_{G,S}FEATURES")
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
ethtool.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ethtool.c b/ethtool.c
index 2fd3393ca5f6..840a5da157a9 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -2430,6 +2430,8 @@ static int do_sfeatures(struct cmd_context *ctx)
err:
free(defs);
+ if (efeatures)
+ free(efeatures);
return rc;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 6/7] ethtool: fix leakage of strings resources in do_sprivflags
2016-09-30 19:56 [PATCH 0/7] ethtool: misc fixes for Coverity issues John W. Linville
` (4 preceding siblings ...)
2016-09-30 19:56 ` [PATCH 5/7] ethtool: fix leakage of efeatures " John W. Linville
@ 2016-09-30 19:56 ` John W. Linville
2016-09-30 19:56 ` [PATCH 7/7] ethtool: fix leakage of strings resources in get_feature_defs John W. Linville
2016-09-30 20:26 ` [PATCH 0/7] ethtool: misc fixes for Coverity issues Greg
7 siblings, 0 replies; 12+ messages in thread
From: John W. Linville @ 2016-09-30 19:56 UTC (permalink / raw)
To: netdev; +Cc: John W. Linville
Coverity issue: 1363124
Fixes: e1ee596326ae ("Add support for querying and setting private flags")
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
ethtool.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/ethtool.c b/ethtool.c
index 840a5da157a9..406cfd2f95a5 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4285,7 +4285,7 @@ static int do_sprivflags(struct cmd_context *ctx)
struct cmdline_info *cmdline;
struct ethtool_value flags;
u32 wanted_flags = 0, seen_flags = 0;
- int any_changed;
+ int any_changed, rc;
unsigned int i;
strings = get_stringset(ctx, ETH_SS_PRIV_FLAGS,
@@ -4297,7 +4297,8 @@ static int do_sprivflags(struct cmd_context *ctx)
}
if (strings->len == 0) {
fprintf(stderr, "No private flags defined\n");
- return 1;
+ rc = 1;
+ goto err;
}
if (strings->len > 32) {
/* ETHTOOL_{G,S}PFLAGS can only cover 32 flags */
@@ -4308,7 +4309,8 @@ static int do_sprivflags(struct cmd_context *ctx)
cmdline = calloc(strings->len, sizeof(*cmdline));
if (!cmdline) {
perror("Cannot parse arguments");
- return 1;
+ rc = 1;
+ goto err;
}
for (i = 0; i < strings->len; i++) {
cmdline[i].name = ((const char *)strings->data +
@@ -4324,17 +4326,22 @@ static int do_sprivflags(struct cmd_context *ctx)
flags.cmd = ETHTOOL_GPFLAGS;
if (send_ioctl(ctx, &flags)) {
perror("Cannot get private flags");
- return 1;
+ rc = 1;
+ goto err;
}
flags.cmd = ETHTOOL_SPFLAGS;
flags.data = (flags.data & ~seen_flags) | wanted_flags;
if (send_ioctl(ctx, &flags)) {
perror("Cannot set private flags");
- return 1;
+ rc = 1;
+ goto err;
}
- return 0;
+ rc = 0;
+err:
+ free(strings);
+ return rc;
}
static int do_tsinfo(struct cmd_context *ctx)
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 7/7] ethtool: fix leakage of strings resources in get_feature_defs
2016-09-30 19:56 [PATCH 0/7] ethtool: misc fixes for Coverity issues John W. Linville
` (5 preceding siblings ...)
2016-09-30 19:56 ` [PATCH 6/7] ethtool: fix leakage of strings resources in do_sprivflags John W. Linville
@ 2016-09-30 19:56 ` John W. Linville
2016-09-30 20:26 ` [PATCH 0/7] ethtool: misc fixes for Coverity issues Greg
7 siblings, 0 replies; 12+ messages in thread
From: John W. Linville @ 2016-09-30 19:56 UTC (permalink / raw)
To: netdev; +Cc: John W. Linville
Coverity issue: 1363125
Fixes: 6042804cf6ec ("Change -k/-K options to use ETHTOOL_{G,S}FEATURES")
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
ethtool.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/ethtool.c b/ethtool.c
index 406cfd2f95a5..5b83a0041a95 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1715,8 +1715,10 @@ static struct feature_defs *get_feature_defs(struct cmd_context *ctx)
}
defs = malloc(sizeof(*defs) + sizeof(defs->def[0]) * n_features);
- if (!defs)
+ if (!defs) {
+ free(names);
return NULL;
+ }
defs->n_features = n_features;
memset(defs->off_flag_matched, 0, sizeof(defs->off_flag_matched));
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/7] ethtool: misc fixes for Coverity issues
2016-09-30 19:56 [PATCH 0/7] ethtool: misc fixes for Coverity issues John W. Linville
` (6 preceding siblings ...)
2016-09-30 19:56 ` [PATCH 7/7] ethtool: fix leakage of strings resources in get_feature_defs John W. Linville
@ 2016-09-30 20:26 ` Greg
7 siblings, 0 replies; 12+ messages in thread
From: Greg @ 2016-09-30 20:26 UTC (permalink / raw)
To: John W. Linville; +Cc: netdev
On Fri, 2016-09-30 at 15:56 -0400, John W. Linville wrote:
> I requested a Coverity scan of the current ethtool sources. There
> weren't many issues identified, but it was non-zero.
>
> Most/all of the resource leak issues can be explained away as
> unimportant due to the fact that ethtool invocations are short-term
> processes. Nevertheless, I prefer to have those issues "fixed" both
> to avoid them as distractions and just to promote better coding
> practices overall.
The patch series looks good to me.
Reviewed-by: Greg Rose <grose@lightfleet.com>
>
> These are "fresh off the press" -- I'll re-review them again before
> applying and pushing them. I appreciate any extra eyeballs that can
> review them as well.
>
> John
> --
> John W. Linville Someday the world will need a hero, and you
> linville@tuxdriver.com might be all we have. Be ready.
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/7] ethtool: avoid resource leak of strings in do_gprivflags
2016-09-30 19:56 ` [PATCH 2/7] ethtool: avoid resource leak of strings in do_gprivflags John W. Linville
@ 2016-10-03 17:54 ` Jarod Wilson
2016-10-04 14:02 ` John W. Linville
2016-10-04 14:05 ` [PATCH v2 " John W. Linville
1 sibling, 1 reply; 12+ messages in thread
From: Jarod Wilson @ 2016-10-03 17:54 UTC (permalink / raw)
To: John W. Linville; +Cc: netdev
On Fri, Sep 30, 2016 at 03:56:16PM -0400, John W. Linville wrote:
> Coverity issue: 1363119
> Fixes: e1ee596326ae ("Add support for querying and setting private flags")
>
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
> ---
> ethtool.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/ethtool.c b/ethtool.c
> index aa3ef5ed2f75..0885a61097ad 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -4205,7 +4205,7 @@ static int do_gprivflags(struct cmd_context *ctx)
> struct ethtool_gstrings *strings;
> struct ethtool_value flags;
> unsigned int i;
> - int max_len = 0, cur_len;
> + int max_len = 0, cur_len, rc;
>
> if (ctx->argc != 0)
> exit_bad_args();
> @@ -4215,11 +4215,13 @@ static int do_gprivflags(struct cmd_context *ctx)
> 1);
> if (!strings) {
> perror("Cannot get private flag names");
> - return 1;
> + rc = 1;
> + goto err;
This goto looks redundant, since all you're doing at err is re-checking if
strings is non-null to free it.
> if (strings->len == 0) {
> fprintf(stderr, "No private flags defined\n");
> - return 1;
> + rc = 1;
> + goto err;
> }
> if (strings->len > 32) {
> /* ETHTOOL_GPFLAGS can only cover 32 flags */
> @@ -4230,7 +4232,8 @@ static int do_gprivflags(struct cmd_context *ctx)
> flags.cmd = ETHTOOL_GPFLAGS;
> if (send_ioctl(ctx, &flags)) {
> perror("Cannot get private flags");
> - return 1;
> + rc = 1;
> + goto err;
> }
>
> /* Find longest string and align all strings accordingly */
> @@ -4248,7 +4251,12 @@ static int do_gprivflags(struct cmd_context *ctx)
> (const char *)strings->data + i * ETH_GSTRING_LEN,
> (flags.data & (1U << i)) ? "on" : "off");
>
> - return 0;
> + rc = 0;
> +
> +err:
> + if (strings)
> + free(strings);
> + return rc;
> }
>
> static int do_sprivflags(struct cmd_context *ctx)
> --
> 2.7.4
>
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/7] ethtool: avoid resource leak of strings in do_gprivflags
2016-10-03 17:54 ` Jarod Wilson
@ 2016-10-04 14:02 ` John W. Linville
0 siblings, 0 replies; 12+ messages in thread
From: John W. Linville @ 2016-10-04 14:02 UTC (permalink / raw)
To: Jarod Wilson; +Cc: netdev
On Mon, Oct 03, 2016 at 01:54:26PM -0400, Jarod Wilson wrote:
> On Fri, Sep 30, 2016 at 03:56:16PM -0400, John W. Linville wrote:
> > Coverity issue: 1363119
> > Fixes: e1ee596326ae ("Add support for querying and setting private flags")
> >
> > Signed-off-by: John W. Linville <linville@tuxdriver.com>
> > ---
> > ethtool.c | 18 +++++++++++++-----
> > 1 file changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/ethtool.c b/ethtool.c
> > index aa3ef5ed2f75..0885a61097ad 100644
> > --- a/ethtool.c
> > +++ b/ethtool.c
> > @@ -4205,7 +4205,7 @@ static int do_gprivflags(struct cmd_context *ctx)
> > struct ethtool_gstrings *strings;
> > struct ethtool_value flags;
> > unsigned int i;
> > - int max_len = 0, cur_len;
> > + int max_len = 0, cur_len, rc;
> >
> > if (ctx->argc != 0)
> > exit_bad_args();
> > @@ -4215,11 +4215,13 @@ static int do_gprivflags(struct cmd_context *ctx)
> > 1);
> > if (!strings) {
> > perror("Cannot get private flag names");
> > - return 1;
> > + rc = 1;
> > + goto err;
>
> This goto looks redundant, since all you're doing at err is re-checking if
> strings is non-null to free it.
True, the original return is just as good there. And since strings
is non-NULL for everything after that, the NULL check at error can
be eliminated as well...
Thanks!
John
> > if (strings->len == 0) {
> > fprintf(stderr, "No private flags defined\n");
> > - return 1;
> > + rc = 1;
> > + goto err;
> > }
> > if (strings->len > 32) {
> > /* ETHTOOL_GPFLAGS can only cover 32 flags */
> > @@ -4230,7 +4232,8 @@ static int do_gprivflags(struct cmd_context *ctx)
> > flags.cmd = ETHTOOL_GPFLAGS;
> > if (send_ioctl(ctx, &flags)) {
> > perror("Cannot get private flags");
> > - return 1;
> > + rc = 1;
> > + goto err;
> > }
> >
> > /* Find longest string and align all strings accordingly */
> > @@ -4248,7 +4251,12 @@ static int do_gprivflags(struct cmd_context *ctx)
> > (const char *)strings->data + i * ETH_GSTRING_LEN,
> > (flags.data & (1U << i)) ? "on" : "off");
> >
> > - return 0;
> > + rc = 0;
> > +
> > +err:
> > + if (strings)
> > + free(strings);
> > + return rc;
> > }
> >
> > static int do_sprivflags(struct cmd_context *ctx)
> > --
> > 2.7.4
> >
>
> --
> Jarod Wilson
> jarod@redhat.com
>
>
>
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/7] ethtool: avoid resource leak of strings in do_gprivflags
2016-09-30 19:56 ` [PATCH 2/7] ethtool: avoid resource leak of strings in do_gprivflags John W. Linville
2016-10-03 17:54 ` Jarod Wilson
@ 2016-10-04 14:05 ` John W. Linville
1 sibling, 0 replies; 12+ messages in thread
From: John W. Linville @ 2016-10-04 14:05 UTC (permalink / raw)
To: netdev; +Cc: Jarod Wilson, John W. Linville
Coverity issue: 1363119
Fixes: e1ee596326ae ("Add support for querying and setting private flags")
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Reviewed-by: Greg Rose <grose@lightfleet.com>
---
ethtool.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/ethtool.c b/ethtool.c
index aa3ef5ed2f75..7ce5667fcdd7 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4205,7 +4205,7 @@ static int do_gprivflags(struct cmd_context *ctx)
struct ethtool_gstrings *strings;
struct ethtool_value flags;
unsigned int i;
- int max_len = 0, cur_len;
+ int max_len = 0, cur_len, rc;
if (ctx->argc != 0)
exit_bad_args();
@@ -4219,7 +4219,8 @@ static int do_gprivflags(struct cmd_context *ctx)
}
if (strings->len == 0) {
fprintf(stderr, "No private flags defined\n");
- return 1;
+ rc = 1;
+ goto err;
}
if (strings->len > 32) {
/* ETHTOOL_GPFLAGS can only cover 32 flags */
@@ -4230,7 +4231,8 @@ static int do_gprivflags(struct cmd_context *ctx)
flags.cmd = ETHTOOL_GPFLAGS;
if (send_ioctl(ctx, &flags)) {
perror("Cannot get private flags");
- return 1;
+ rc = 1;
+ goto err;
}
/* Find longest string and align all strings accordingly */
@@ -4248,7 +4250,11 @@ static int do_gprivflags(struct cmd_context *ctx)
(const char *)strings->data + i * ETH_GSTRING_LEN,
(flags.data & (1U << i)) ? "on" : "off");
- return 0;
+ rc = 0;
+
+err:
+ free(strings);
+ return rc;
}
static int do_sprivflags(struct cmd_context *ctx)
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-10-04 14:15 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-30 19:56 [PATCH 0/7] ethtool: misc fixes for Coverity issues John W. Linville
2016-09-30 19:56 ` [PATCH 1/7] ethtool: avoid NULL pointer dereference in do_permaddr John W. Linville
2016-09-30 19:56 ` [PATCH 2/7] ethtool: avoid resource leak of strings in do_gprivflags John W. Linville
2016-10-03 17:54 ` Jarod Wilson
2016-10-04 14:02 ` John W. Linville
2016-10-04 14:05 ` [PATCH v2 " John W. Linville
2016-09-30 19:56 ` [PATCH 3/7] ethtool: plug resource leaks of defs and features in do_gfeatures John W. Linville
2016-09-30 19:56 ` [PATCH 4/7] ethtool: fix leakage of defs resources in do_sfeatures John W. Linville
2016-09-30 19:56 ` [PATCH 5/7] ethtool: fix leakage of efeatures " John W. Linville
2016-09-30 19:56 ` [PATCH 6/7] ethtool: fix leakage of strings resources in do_sprivflags John W. Linville
2016-09-30 19:56 ` [PATCH 7/7] ethtool: fix leakage of strings resources in get_feature_defs John W. Linville
2016-09-30 20:26 ` [PATCH 0/7] ethtool: misc fixes for Coverity issues Greg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).