[ofa-general] [PATCH] IB: Possible write outside array bounds

[Hal Rosenstock]

On Wed, Jul 29, 2009 at 3:15 PM, Roland Dreier <rdreier at cisco.com> wrote:

>
>  > Based on the spec limiting hop pointer to 255 and not 63, I think the
>  > above should just be a check on hop count and not hop pointer:
>  >             if (hop_cnt >= IB_SMP_MAX_PATH_HOPS)
>
> Yes, it seems that the current code then properly checks hop_ptr against
> hop_cnt in all cases.  Do we all agree that the following patch is
> right?  If so I'll queue it for 2.6.32:
>
>  drivers/infiniband/core/smi.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/infiniband/core/smi.c b/drivers/infiniband/core/smi.c
> index 8723675..a10152d 100644
> --- a/drivers/infiniband/core/smi.c
> +++ b/drivers/infiniband/core/smi.c
> @@ -52,6 +52,10 @@ enum smi_action smi_handle_dr_smp_send(struct ib_smp
> *smp,
>        hop_cnt = smp->hop_cnt;
>
>        /* See section 14.2.2.2, Vol 1 IB spec */
> +       /* C14-6 -- valid hop_cnt values are from 0 to 63 */
> +       if (hop_cnt >= IB_SMP_MAX_PATH_HOPS)
> +               return IB_SMI_DISCARD;
> +
>        if (!ib_get_smp_direction(smp)) {
>                /* C14-9:1 */
>                if (hop_cnt && hop_ptr == 0) {
>

That looks right to me on the send side. Shouldn't there be the same check
on the recv side (smi_handle_dr_smp_recv) which was the intent of Roel's
original patch ?

-- Hal
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/general/attachments/20090729/c338dd81/attachment.html>