| Message ID | 1498835756-7610-1-git-send-email-danielj@mellanox.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Fri, Jun 30, 2017 at 11:15 AM, Dan Jurgens <danielj@mellanox.com> wrote: > From: Daniel Jurgens <danielj@mellanox.com> > > ib_get_cached_subnet_prefix can technically fail, but the only way it > could is not possible based on the loop conditions. Check the return > value before using the variable sp to resolve a static analysis warning. > > Fixes: 8f408ab64be6 ("selinux lsm IB/core: Implement LSM notification > system") > Signed-off-by: Daniel Jurgens <danielj@mellanox.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/infiniband/core/device.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Similar question as with the other fix: SELinux tree or IB/RDMA tree?
On 7/1/2017 9:42 AM, Paul Moore wrote: > On Fri, Jun 30, 2017 at 11:15 AM, Dan Jurgens <danielj@mellanox.com> wrote: >> From: Daniel Jurgens <danielj@mellanox.com> >> >> ib_get_cached_subnet_prefix can technically fail, but the only way it >> could is not possible based on the loop conditions. Check the return >> value before using the variable sp to resolve a static analysis warning. >> >> Fixes: 8f408ab64be6 ("selinux lsm IB/core: Implement LSM notification >> system") >> Signed-off-by: Daniel Jurgens <danielj@mellanox.com> >> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> >> --- >> drivers/infiniband/core/device.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) > > Similar question as with the other fix: SELinux tree or IB/RDMA tree? > The SELinux tree will be fine with me.
On Fri, Jun 30, 2017 at 11:15 AM, Dan Jurgens <danielj@mellanox.com> wrote: > From: Daniel Jurgens <danielj@mellanox.com> > > ib_get_cached_subnet_prefix can technically fail, but the only way it > could is not possible based on the loop conditions. Check the return > value before using the variable sp to resolve a static analysis warning. > > Fixes: 8f408ab64be6 ("selinux lsm IB/core: Implement LSM notification > system") > Signed-off-by: Daniel Jurgens <danielj@mellanox.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/infiniband/core/device.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c > index 631eaa9..dabd9f9 100644 > --- a/drivers/infiniband/core/device.c > +++ b/drivers/infiniband/core/device.c > @@ -376,7 +376,8 @@ static void ib_policy_change_task(struct work_struct *work) > WARN_ONCE(ret, > "ib_get_cached_subnet_prefix err: %d, this should never happen here\n", > ret); > - ib_security_cache_change(dev, i, sp); > + if (ret) Should this be "if (!ret)"? > + ib_security_cache_change(dev, i, sp); > } > } > up_read(&lists_rwsem);
On Mon, Jul 03, 2017 at 07:03:54PM -0400, Paul Moore wrote: > On Fri, Jun 30, 2017 at 11:15 AM, Dan Jurgens <danielj@mellanox.com> wrote: > > From: Daniel Jurgens <danielj@mellanox.com> > > > > ib_get_cached_subnet_prefix can technically fail, but the only way it > > could is not possible based on the loop conditions. Check the return > > value before using the variable sp to resolve a static analysis warning. > > > > Fixes: 8f408ab64be6 ("selinux lsm IB/core: Implement LSM notification > > system") > > Signed-off-by: Daniel Jurgens <danielj@mellanox.com> > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > drivers/infiniband/core/device.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c > > index 631eaa9..dabd9f9 100644 > > --- a/drivers/infiniband/core/device.c > > +++ b/drivers/infiniband/core/device.c > > @@ -376,7 +376,8 @@ static void ib_policy_change_task(struct work_struct *work) > > WARN_ONCE(ret, > > "ib_get_cached_subnet_prefix err: %d, this should never happen here\n", > > ret); > > - ib_security_cache_change(dev, i, sp); > > + if (ret) > > Should this be "if (!ret)"? You are right, It should. Thanks > > > + ib_security_cache_change(dev, i, sp); > > } > > } > > up_read(&lists_rwsem); > > -- > paul moore > www.paul-moore.com > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/3/2017 6:03 PM, Paul Moore wrote: > On Fri, Jun 30, 2017 at 11:15 AM, Dan Jurgens <danielj@mellanox.com> wrote: >> From: Daniel Jurgens <danielj@mellanox.com> >> >> ib_get_cached_subnet_prefix can technically fail, but the only way it >> could is not possible based on the loop conditions. Check the return >> value before using the variable sp to resolve a static analysis warning. >> >> Fixes: 8f408ab64be6 ("selinux lsm IB/core: Implement LSM notification >> system") >> Signed-off-by: Daniel Jurgens <danielj@mellanox.com> >> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> >> --- >> drivers/infiniband/core/device.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c >> index 631eaa9..dabd9f9 100644 >> --- a/drivers/infiniband/core/device.c >> +++ b/drivers/infiniband/core/device.c >> @@ -376,7 +376,8 @@ static void ib_policy_change_task(struct work_struct *work) >> WARN_ONCE(ret, >> "ib_get_cached_subnet_prefix err: %d, this should never happen here\n", >> ret); >> - ib_security_cache_change(dev, i, sp); >> + if (ret) > Should this be "if (!ret)"? Yes, my apologies. I missed the git add after fixing that locally. I sent v1 a minute ago. > >> + ib_security_cache_change(dev, i, sp); >> } >> } >> up_read(&lists_rwsem);
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c index 631eaa9..dabd9f9 100644 --- a/drivers/infiniband/core/device.c +++ b/drivers/infiniband/core/device.c @@ -376,7 +376,8 @@ static void ib_policy_change_task(struct work_struct *work) WARN_ONCE(ret, "ib_get_cached_subnet_prefix err: %d, this should never happen here\n", ret); - ib_security_cache_change(dev, i, sp); + if (ret) + ib_security_cache_change(dev, i, sp); } } up_read(&lists_rwsem);