[PATCH] IB/core: Fix static analysis warning in ib_policy_change_task

[PATCH] IB/core: Fix static analysis warning in ib_policy_change_task

[Dan Jurgens]

From: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

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-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reported-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 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)
+				ib_security_cache_change(dev, i, sp);
 		}
 	}
 	up_read(&lists_rwsem);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] IB/core: Fix static analysis warning in ib_policy_change_task

[Paul Moore]

On Fri, Jun 30, 2017 at 11:15 AM, Dan Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> From: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> 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-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reported-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
>  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?

-- 
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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] IB/core: Fix static analysis warning in ib_policy_change_task

[Doug Ledford]

[-- Attachment #1.1: Type: text/plain, Size: 1122 bytes --]

On 7/1/2017 9:42 AM, Paul Moore wrote:
> On Fri, Jun 30, 2017 at 11:15 AM, Dan Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> From: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>
>> 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-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> Reported-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>> ---
>>  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.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG Key ID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: [PATCH] IB/core: Fix static analysis warning in ib_policy_change_task

[Paul Moore]

On Fri, Jun 30, 2017 at 11:15 AM, Dan Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> From: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> 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-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reported-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
>  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);

-- 
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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] IB/core: Fix static analysis warning in ib_policy_change_task

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1911 bytes --]

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-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> > From: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > 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-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Reported-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > ---
> >  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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] IB/core: Fix static analysis warning in ib_policy_change_task

[Daniel Jurgens]

On 7/3/2017 6:03 PM, Paul Moore wrote:
> On Fri, Jun 30, 2017 at 11:15 AM, Dan Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> From: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>
>> 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-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> Reported-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>> ---
>>  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);


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html