[PATCH] opensm/osm_torus.c: Fix crash in torus_update_osm_vlarb

[PATCH] opensm/osm_torus.c: Fix crash in torus_update_osm_vlarb

[Hal Rosenstock]

Signed-off-by: Alex Netes <alexne-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 opensm/osm_torus.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/opensm/osm_torus.c b/opensm/osm_torus.c
index c06f8d4..075f84a 100644
--- a/opensm/osm_torus.c
+++ b/opensm/osm_torus.c
@@ -8089,7 +8089,7 @@ void torus_update_osm_vlarb(void *context, osm_physp_t *osm_phys_port,
 	 * So, leave VL 0 alone, remap VL 4 to VL 1, zero out the rest,
 	 * and compress out the zero entries to the end.
 	 */
-	if (!sw || !port_num ||
+	if (!sw || !port_num || sw->port[port_num] ||
 	    sw->port[port_num]->pgrp->port_grp != 2 * TORUS_MAX_DIM)
 		return;
  
-- 
1.7.8.2

--
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] opensm/osm_torus.c: Fix crash in torus_update_osm_vlarb

[Jim Schutt]

On 12/03/2012 08:26 AM, Hal Rosenstock wrote:
>
> Signed-off-by: Alex Netes<alexne-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Hal Rosenstock<hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>   opensm/osm_torus.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/opensm/osm_torus.c b/opensm/osm_torus.c
> index c06f8d4..075f84a 100644
> --- a/opensm/osm_torus.c
> +++ b/opensm/osm_torus.c
> @@ -8089,7 +8089,7 @@ void torus_update_osm_vlarb(void *context, osm_physp_t *osm_phys_port,
>   	 * So, leave VL 0 alone, remap VL 4 to VL 1, zero out the rest,
>   	 * and compress out the zero entries to the end.
>   	 */
> -	if (!sw || !port_num ||
> +	if (!sw || !port_num || sw->port[port_num] ||
>   	    sw->port[port_num]->pgrp->port_grp != 2 * TORUS_MAX_DIM)
>   		return;
>

With the patch as-is, if torus_update_osm_vlarb() returns early
for any non-NULL switch port, it will never do any updates.

If the crash was that sw->port[port_num] was NULL,
shouldn't the check be !sw->port[port_num] ?

Can you tell me more about the test case that leads to the crash?

Is it that there's a switch with a port that's not connected
to anything, and torus_update_osm_vlarb() was called for it?

Testing for a non-NULL sw->port[port_num] is definitely the right
thing to do to handle that case, and I'm sorry I missed it earlier.

If not, then something else is likely broken, and we need to find
and fix that.

-- Jim

--
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] opensm/osm_torus.c: Fix crash in torus_update_osm_vlarb

[Hal Rosenstock]

On 12/3/2012 4:09 PM, Jim Schutt wrote:
> On 12/03/2012 08:26 AM, Hal Rosenstock wrote:
>>
>> Signed-off-by: Alex Netes<alexne-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: Hal Rosenstock<hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> ---
>>   opensm/osm_torus.c |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/opensm/osm_torus.c b/opensm/osm_torus.c
>> index c06f8d4..075f84a 100644
>> --- a/opensm/osm_torus.c
>> +++ b/opensm/osm_torus.c
>> @@ -8089,7 +8089,7 @@ void torus_update_osm_vlarb(void *context,
>> osm_physp_t *osm_phys_port,
>>        * So, leave VL 0 alone, remap VL 4 to VL 1, zero out the rest,
>>        * and compress out the zero entries to the end.
>>        */
>> -    if (!sw || !port_num ||
>> +    if (!sw || !port_num || sw->port[port_num] ||
>>           sw->port[port_num]->pgrp->port_grp != 2 * TORUS_MAX_DIM)
>>           return;
>>
> 
> With the patch as-is, if torus_update_osm_vlarb() returns early
> for any non-NULL switch port, it will never do any updates.
> 
> If the crash was that sw->port[port_num] was NULL,
> shouldn't the check be !sw->port[port_num] ?
> 
> Can you tell me more about the test case that leads to the crash?
> 
> Is it that there's a switch with a port that's not connected
> to anything, and torus_update_osm_vlarb() was called for it?
> 
> Testing for a non-NULL sw->port[port_num] is definitely the right
> thing to do to handle that case, and I'm sorry I missed it earlier.
> 
> If not, then something else is likely broken, and we need to find
> and fix that.

Yes, it was meant to be NULL pointer check. v2 of patch coming soon.

-- Hal

> 
> -- Jim
> 
> 

--
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