<br><br>
<div class="gmail_quote">On Sun, Aug 2, 2009 at 6:59 AM, Hal Rosenstock <span dir="ltr"><<a href="mailto:hal.rosenstock@gmail.com">hal.rosenstock@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid">HI Sasha,<br><br>
<div class="gmail_quote">
<div class="im">On Sun, Aug 2, 2009 at 6:32 AM, Sasha Khapyorsky <span dir="ltr"><<a href="mailto:sashak@voltaire.com" target="_blank">sashak@voltaire.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid">Hi Hal,<br>
<div><br>On 09:53 Fri 31 Jul     , Hal Rosenstock wrote:<br>><br>> based on valid/invalid hop count rather than relying on debug assert<br><br></div>When could an invalid hop count be passed to this function? </blockquote>

<div> </div></div>
<div>Some out of tree user.</div></div></blockquote>
<div> </div>
<div>Also, I think opensm can also do this now with such topologies (without some other changes which are in the pipe).</div>
<div> </div>
<div>-- Hal</div>
<div> </div>
<blockquote class="gmail_quote" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid">
<div class="gmail_quote">
<div><span id=""></span></div>
<div> </div>
<blockquote class="gmail_quote" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid"><span></span>And what could happen?</blockquote>
<div> </div>
<div>It writes past the end of the path array.</div>
<div class="im">
<div> </div>
<blockquote class="gmail_quote" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid"><span></span><br>ib_smp_init_new() is a simple structure fill-up helper (inlined and<br>defined in header file) and I don't think that we need to check<br>
parameters there.<br><br>This patch also introduces sort of inconsistency - a hop count is checked<br>and other parameters aren't.</blockquote>
<div> </div></div>
<div>It's to protect against writing past end of array. Do any other parameters need checking ? I think they just result in some timeout condition resulting.</div>
<div> </div><font color="#888888">
<div>-- Hal</div></font>
<div>
<div></div>
<div class="h5">
<div> </div>
<blockquote class="gmail_quote" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid"><span></span><br>
<div>
<div></div>
<div><br>> Handle invalid status appropriate in callers of ib_smp_init_new<br>><br>> Signed-off-by: Hal Rosenstock <<a href="mailto:hal.rosenstock@gmail.com" target="_blank">hal.rosenstock@gmail.com</a>><br>
> ---<br>> diff --git a/opensm/include/iba/ib_types.h b/opensm/include/iba/ib_types.h<br>> index beb7492..6668d96 100644<br>> --- a/opensm/include/iba/ib_types.h<br>> +++ b/opensm/include/iba/ib_types.h<br>
> @@ -4091,11 +4092,11 @@ static inline boolean_t OSM_API ib_smp_is_d(IN const ib_smp_t * const p_smp)<br>>  *<br>>  * TODO<br>>  *    This is too big for inlining, but leave it here for now<br>> -*    since there is not yet another convient spot.<br>
> +*    since there is not yet another convenient spot.<br>>  *<br>>  * SYNOPSIS<br>>  */<br>> -static inline void OSM_API<br>> +static inline boolean_t OSM_API<br>>  ib_smp_init_new(IN ib_smp_t * const p_smp,<br>
>               IN const uint8_t method,<br>>               IN const ib_net64_t trans_id,<br>> @@ -4107,7 +4108,9 @@ ib_smp_init_new(IN ib_smp_t * const p_smp,<br>>               IN const ib_net16_t dr_slid, IN const ib_net16_t dr_dlid)<br>
>  {<br>>       CL_ASSERT(p_smp);<br>> -     CL_ASSERT(hop_count < IB_SUBNET_PATH_HOPS_MAX);<br>> +<br>> +     if (hop_count >= IB_SUBNET_PATH_HOPS_MAX)<br>> +             return FALSE;<br>>       p_smp->base_ver = 1;<br>
>       p_smp->mgmt_class = IB_MCLASS_SUBN_DIR;<br>>       p_smp->class_ver = 1;<br>> @@ -4130,6 +4133,7 @@ ib_smp_init_new(IN ib_smp_t * const p_smp,<br>><br>>       /* copy the path */<br>>       memcpy(&p_smp->initial_path, path_out, sizeof(p_smp->initial_path));<br>
> +     return TRUE;<br>>  }<br>><br>>  /*<br>> diff --git a/opensm/opensm/osm_req.c b/opensm/opensm/osm_req.c<br>> index be9a92b..7934173 100644<br>> --- a/opensm/opensm/osm_req.c<br>> +++ b/opensm/opensm/osm_req.c<br>
> @@ -102,14 +102,21 @@ osm_req_get(IN osm_sm_t * sm,<br>>               ib_get_sm_attr_str(attr_id), cl_ntoh16(attr_id),<br>>               cl_ntoh32(attr_mod), cl_ntoh64(tid));<br>><br>> -     ib_smp_init_new(osm_madw_get_smp_ptr(p_madw),<br>
> -                     IB_MAD_METHOD_GET,<br>> -                     tid,<br>> -                     attr_id,<br>> -                     attr_mod,<br>> -                     p_path->hop_count,<br>> -                     sm->p_subn->opt.m_key,<br>
> -                     p_path->path, IB_LID_PERMISSIVE, IB_LID_PERMISSIVE);<br>> +     if (!ib_smp_init_new(osm_madw_get_smp_ptr(p_madw),<br>> +                          IB_MAD_METHOD_GET,<br>> +                          tid,<br>
> +                          attr_id,<br>> +                          attr_mod,<br>> +                          p_path->hop_count,<br>> +                          sm->p_subn->opt.m_key,<br>> +                          p_path->path,<br>
> +                          IB_LID_PERMISSIVE, IB_LID_PERMISSIVE)) {<br>> +             OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 1108: "<br>> +                     "ib_smp_init_new failed: hop count %d\n",<br>
> +                     p_path->hop_count);<br><br></div></div>This is assumption on how ib_smp_init_new() is actually implemented -<br>not perfect.<br><font color="#888888"><br>Sasha<br></font>
<div>
<div></div>
<div>_______________________________________________<br>general mailing list<br><a href="mailto:general@lists.openfabrics.org" target="_blank">general@lists.openfabrics.org</a><br><a href="http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general" target="_blank">http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general</a><br>
<br>To unsubscribe, please visit <a href="http://openib.org/mailman/listinfo/openib-general" target="_blank">http://openib.org/mailman/listinfo/openib-general</a><br></div></div></blockquote></div></div></div><br></blockquote>
</div><br>