| Message ID | 20210106144343.22099-1-msuchanek@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [ndctl] dimm: re-fix potential fd leakage in dimm_action() | expand |
Michal Suchanek <msuchanek@suse.de> writes: > There are cases not covered by the original fix and cases added by the > latter patch. > > Also there is one case of usage added without returning from the > function. > > Fixes: ff434d87ccbd ("dimm: fix potential fd leakage in dimm_action()") > Fixes: 41a7e24af5db ("ndctl/dimm: Auto-arm firmware activation") > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > --- > ndctl/dimm.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/ndctl/dimm.c b/ndctl/dimm.c > index 09ce49e1d2ca..1c177b5494ec 100644 > --- a/ndctl/dimm.c > +++ b/ndctl/dimm.c > @@ -1333,12 +1333,15 @@ static int dimm_action(int argc, const char **argv, struct ndctl_ctx *ctx, > if (param.arm_set && param.disarm_set) { > fprintf(stderr, "set either --arm, or --disarm, not both\n"); > usage_with_options(u, options); > + rc = -EINVAL; > + goto out_close_fout; usage_with_options calls exit(): void usage_with_options(const char * const *usagestr, const struct option *opts) { usage_with_options_internal(usagestr, opts, 0); exit(129); } So I don't think this patch is necessary. Cheers, Jeff
diff --git a/ndctl/dimm.c b/ndctl/dimm.c index 09ce49e1d2ca..1c177b5494ec 100644 --- a/ndctl/dimm.c +++ b/ndctl/dimm.c @@ -1333,12 +1333,15 @@ static int dimm_action(int argc, const char **argv, struct ndctl_ctx *ctx, if (param.arm_set && param.disarm_set) { fprintf(stderr, "set either --arm, or --disarm, not both\n"); usage_with_options(u, options); + rc = -EINVAL; + goto out_close_fout; } if (param.disarm_set && !param.disarm) { fprintf(stderr, "--no-disarm syntax not supported\n"); usage_with_options(u, options); - return -EINVAL; + rc = -EINVAL; + goto out_close_fout; } if (!param.infile) { @@ -1351,13 +1354,15 @@ static int dimm_action(int argc, const char **argv, struct ndctl_ctx *ctx, if (!param.arm_set && !param.disarm_set) { fprintf(stderr, "require --arm, or --disarm\n"); usage_with_options(u, options); - return -EINVAL; + rc = -EINVAL; + goto out_close_fout; } if (param.arm_set && !param.arm) { fprintf(stderr, "--no-arm syntax not supported\n"); usage_with_options(u, options); - return -EINVAL; + rc = -EINVAL; + goto out_close_fout; } } actx.f_in = stdin; @@ -1425,7 +1430,8 @@ static int dimm_action(int argc, const char **argv, struct ndctl_ctx *ctx, if (count > 1) { error("write-labels only supports writing a single dimm\n"); usage_with_options(u, options); - return -EINVAL; + rc = -EINVAL; + goto out_close_fin_fout; } else if (single) rc = action(single, &actx); }
There are cases not covered by the original fix and cases added by the latter patch. Also there is one case of usage added without returning from the function. Fixes: ff434d87ccbd ("dimm: fix potential fd leakage in dimm_action()") Fixes: 41a7e24af5db ("ndctl/dimm: Auto-arm firmware activation") Signed-off-by: Michal Suchanek <msuchanek@suse.de> --- ndctl/dimm.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)