[ofa-general] [PATCH] opensm/osm_ucast_lash: fix buffer overflow

[Robert Pearson]

Thanks Hal,

I grabbed the head of tree and recompiled with some print statements thrown
in. As you say the '<<' fixes the decoding correctly and ibsim is reporting
4 not 1 so life is good and I see no problems in ibsim or opensm.

I was looking at the binary init values for port info for switches and it
looked like the op_vl was set to 1 but when I ran opensm over it I saw 4
which is the correct answer. The last point is moot in this case.

I artificially relaxed the limit on the number of VLs in my case and found
that I needed 12 VLs to route my problem with lash so it is not going to
work since the switches only support 8 data vls.

Regards,

Bob
-----Original Message-----
From: Hal Rosenstock [mailto:hal.rosenstock at gmail.com] 
Sent: Monday, October 13, 2008 11:58 AM
To: Robert Pearson
Cc: Sasha Khapyorsky; OpenIB
Subject: Re: [ofa-general] [PATCH] opensm/osm_ucast_lash: fix buffer
overflow

On Sun, Oct 12, 2008 at 7:55 PM, Robert Pearson
<rpearson at systemfabricworks.com> wrote:
> I spent a little time looking at osm_ucast_lash.c and ibsim.
>
> It looks like ibsim reports vl_cap = 4 and op_vl = 1 by default for a
> switch.

Yes, ibsim only has one canned template for this. I think it is mainly
that ibnetdiscover output format doesn't include this information
currently and something needs to be assumed.

> Osm_ucast_lash.c computes the minimum over all switches of op_vl as
> extracted from the port info mads starting from (5 which would correspond
to
>
> It then uses the encoded value as though it was an integer instead of an
> encoding of an integer which seems wrong.

osm_ucast_lash.c::discover_network_properties fixes this up properly:
      vl_min = 1 << (vl_min - 1);
      if (vl_min > 15)
              vl_min = 15;

> I am not yet sure when the SM is supposed to set the op_vl field away from
> 1. If later then you are using the wrong value and should be comparing to
> the decoded value of vl_cap instead.

I don't understand what you mean here. vl_cap would never be right to be
used.

-- Hal

> -----Original Message-----
> From: general-bounces at lists.openfabrics.org
> [mailto:general-bounces at lists.openfabrics.org] On Behalf Of Robert Pearson
> Sent: Sunday, October 12, 2008 5:21 PM
> To: 'Sasha Khapyorsky'; 'Hal Rosenstock'
> Cc: 'OpenIB'
> Subject: RE: [ofa-general] [PATCH] opensm/osm_ucast_lash: fix buffer
> overflow
>
> How does lash know how many VLs are available? Especially, with ibsim.
> Is there a way to have lash report the number of VLs required independent
of
> the type of switch used?
>
> -----Original Message-----
> From: general-bounces at lists.openfabrics.org
> [mailto:general-bounces at lists.openfabrics.org] On Behalf Of Sasha
Khapyorsky
> Sent: Thursday, October 09, 2008 2:06 PM
> To: Hal Rosenstock
> Cc: OpenIB
> Subject: Re: [ofa-general] [PATCH] opensm/osm_ucast_lash: fix buffer
> overflow
>
> On 07:04 Wed 08 Oct     , Hal Rosenstock wrote:
>>
>> Minor simplification as it seems like this could just be:
>>
>> if (++lanes_needed > p_lash->vl_min)
>>   goto Error_Not_Enough_Lanes;
>
> Works for me. Thanks!
>
> Sasha
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>
> To unsubscribe, please visit
> http://openib.org/mailman/listinfo/openib-general
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>
> To unsubscribe, please visit
> http://openib.org/mailman/listinfo/openib-general
>