| Message ID | 20220823074527.404435-2-vishal.l.verma@intel.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | cxl: static analysis fixes | expand |
Vishal Verma wrote: > A NULL check in region_action() implies that 'decoder' might be NULL, but > later we dereference it during cxl_decoder_foreach(). The NULL check is > valid because it was the filter result being checked, however, while > doing this, the original 'decoder' variable was being clobbered. > > Check the filter results independently of the original decoder variable. > > Cc: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > --- > cxl/region.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/cxl/region.c b/cxl/region.c > index a30313c..334fcc2 100644 > --- a/cxl/region.c > +++ b/cxl/region.c > @@ -686,9 +686,8 @@ static int region_action(int argc, const char **argv, struct cxl_ctx *ctx, > continue; > > cxl_decoder_foreach (port, decoder) { > - decoder = util_cxl_decoder_filter(decoder, > - param.root_decoder); > - if (!decoder) > + if (!util_cxl_decoder_filter(decoder, > + param.root_decoder)) > continue; Looks good. Reviewed-by: Dan Williams <dan.j.williams@intel.com>
On 8/23/2022 12:45 AM, Vishal Verma wrote: > A NULL check in region_action() implies that 'decoder' might be NULL, but > later we dereference it during cxl_decoder_foreach(). The NULL check is > valid because it was the filter result being checked, however, while > doing this, the original 'decoder' variable was being clobbered. > > Check the filter results independently of the original decoder variable. > > Cc: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> > --- > cxl/region.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/cxl/region.c b/cxl/region.c > index a30313c..334fcc2 100644 > --- a/cxl/region.c > +++ b/cxl/region.c > @@ -686,9 +686,8 @@ static int region_action(int argc, const char **argv, struct cxl_ctx *ctx, > continue; > > cxl_decoder_foreach (port, decoder) { > - decoder = util_cxl_decoder_filter(decoder, > - param.root_decoder); > - if (!decoder) > + if (!util_cxl_decoder_filter(decoder, > + param.root_decoder)) > continue; > rc = decoder_region_action(p, decoder, action, count); > if (rc)
On Tue, 23 Aug 2022 01:45:25 -0600 Vishal Verma <vishal.l.verma@intel.com> wrote: > A NULL check in region_action() implies that 'decoder' might be NULL, but > later we dereference it during cxl_decoder_foreach(). The NULL check is > valid because it was the filter result being checked, however, while > doing this, the original 'decoder' variable was being clobbered. > > Check the filter results independently of the original decoder variable. > > Cc: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> Typo in patch title. > --- > cxl/region.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/cxl/region.c b/cxl/region.c > index a30313c..334fcc2 100644 > --- a/cxl/region.c > +++ b/cxl/region.c > @@ -686,9 +686,8 @@ static int region_action(int argc, const char **argv, struct cxl_ctx *ctx, > continue; > > cxl_decoder_foreach (port, decoder) { > - decoder = util_cxl_decoder_filter(decoder, > - param.root_decoder); > - if (!decoder) > + if (!util_cxl_decoder_filter(decoder, > + param.root_decoder)) > continue; > rc = decoder_region_action(p, decoder, action, count); > if (rc)
diff --git a/cxl/region.c b/cxl/region.c index a30313c..334fcc2 100644 --- a/cxl/region.c +++ b/cxl/region.c @@ -686,9 +686,8 @@ static int region_action(int argc, const char **argv, struct cxl_ctx *ctx, continue; cxl_decoder_foreach (port, decoder) { - decoder = util_cxl_decoder_filter(decoder, - param.root_decoder); - if (!decoder) + if (!util_cxl_decoder_filter(decoder, + param.root_decoder)) continue; rc = decoder_region_action(p, decoder, action, count); if (rc)
A NULL check in region_action() implies that 'decoder' might be NULL, but later we dereference it during cxl_decoder_foreach(). The NULL check is valid because it was the filter result being checked, however, while doing this, the original 'decoder' variable was being clobbered. Check the filter results independently of the original decoder variable. Cc: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- cxl/region.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)