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

[Hal Rosenstock]

On Fri, Jul 24, 2009 at 7:30 AM, Roel Kluin<roel.kluin at gmail.com> wrote:
> Ensure index stays within smp->return_path[] and ->initial_path[].
>
> Signed-off-by: Roel Kluin <roel.kluin at gmail.com>
> ---
> This was observed using Parfait (http://research.sun.com/projects/parfait/)
>
> It appears that hop_ptr may be able to range up to 255, potentially writing
> outside the buffer.
>
> diff --git a/drivers/infiniband/core/smi.c b/drivers/infiniband/core/smi.c
> index 8723675..8303b80 100644
> --- a/drivers/infiniband/core/smi.c
> +++ b/drivers/infiniband/core/smi.c
> @@ -140,7 +140,8 @@ enum smi_action smi_handle_dr_smp_recv(struct ib_smp *smp, u8 node_type,
>
>                /* C14-9:2 -- intermediate hop */
>                if (hop_ptr && hop_ptr < hop_cnt) {
> -                       if (node_type != RDMA_NODE_IB_SWITCH)
> +                       if (node_type != RDMA_NODE_IB_SWITCH ||
> +                                       hop_ptr >= IB_SMP_MAX_PATH_HOPS - 1)

Yes, nice catch. This appears correct but I think the issue of hop
path/count validation is slightly larger. In that routine
(smi_handle_dr_smp_recv), I would think that if either hop_ptr or
hop_cnt are >= IB_SMP_MAX_PATH_HOPS then IB_SMI_DISCARD should be
returned. If that validation is done at the top of that routine, then
the hop_ptr < hop_cnt check in this C14-9:2 case eliminates the need
to special case this here. Make sense ?

-- Hal

>                                return IB_SMI_DISCARD;
>
>                        smp->return_path[hop_ptr] = port_num;
> _______________________________________________
> 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
>