On Tue, Oct 29, 2019 at 04:33:27PM -0700, 'Ira Weiny' wrote:
> On Tue, Oct 29, 2019 at 08:27:36AM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> >
> > All callers allocate MAD structures with proper sizes,
> > there is no need to recheck it.
> >
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > ---
> > drivers/infiniband/hw/hfi1/mad.c | 4 ----
> > 1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/hfi1/mad.c b/drivers/infiniband/hw/hfi1/mad.c
> > index d8ff063a5419..a54746f4a0ae 100644
> > --- a/drivers/infiniband/hw/hfi1/mad.c
> > +++ b/drivers/infiniband/hw/hfi1/mad.c
> > @@ -4921,10 +4921,6 @@ int hfi1_process_mad(struct ib_device *ibdev, int mad_flags, u8 port,
> > {
> > switch (in_mad->base_version) {
> > case OPA_MGMT_BASE_VERSION:
> > - if (unlikely(in_mad_size != sizeof(struct opa_mad))) {
> > - dev_err(ibdev->dev.parent, "invalid in_mad_size\n");
> > - return IB_MAD_RESULT_FAILURE;
> > - }
>
> It's been a while but I'm not 100% sure we can safely remove this check. A
> user can send an IB sized MAD to an OPA device and AFAIR there is nothing
> checking that the base version is set correctly for the size of mad received by
> the mad stack.
>
> Are you 100% sure that the in_mad_size is based off the management base
> version?
Ok, I _think_ this check may actually be wrong and we were just lucky because
handle_outgoing_dr_smp() uses the port _max_ MAD size as the in_mad_size which is
not _technically_ correct. And in normal RX we are passing 2K as
in_mad_size... Again _not_ technically correct but irrelevant because it looks
like we pass the actual bytes received in mad_priv->header.recv_wc->mad_len.
After looking at your last patch I started to remember more. OPA MADs are
variable size so this was the reasoning for the addition of the in/out size
parameters.
But I see that nothing in the hfi1 driver is actually using the in_mad_size.
So I think we may be ok here but...
Denny, would be nice to get this patch set tested though... Just to make sure.
Thanks, and sorry for the noise.
Ira
>
> Also, regarding the patches to remove the checks on the IB devices I would want
> to see that check in the core MAD stack to verify we are not sending an OPA mad
> to an IB device. I may have put a check in there already so removing the code
> in those drivers may be ok. I'm not sure off the top of my head...
>
> Ira
>
> > return hfi1_process_opa_mad(ibdev, mad_flags, port,
> > in_wc, in_grh,
> > (struct opa_mad *)in_mad,
> > --
> > 2.20.1
> >
