Re: [PATCH rdma-next v1 09/13] IB/cm: Avoid AV ah_attr overwriting during LAP message handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 14, 2018 at 07:36:07PM -0600, Jason Gunthorpe wrote:
> On Thu, Mar 15, 2018 at 12:01:05AM +0000, Parav Pandit wrote:
>
> > > So we can call something like ib_send_cm_req from userspace and race it with
> > > the above code, and corrupt av. Not sure why ibv_send_cm_req doesn't hold the
> > > spinlock while calling cm_init_av_by_path, probably should.
>
> > AV initialization is blocking call.
>
> Well, I found call sites that call cm_init_av_by_path under a
> spinlock :(
>
> > > I think all the places that write to the av need to update the av under the spin
> > > lock. Looks like the best way to do it above is to write a temporary av to the
> > > stack and then re-acquire the lock and copy it to the priv ?? All places that write
> > > to av need checking..
>
> > This is done for non-LAP message processing path in internal series
> > under review to you, Leon, Mark for a while now.  Since you are
> > asking this more high level question, I like to ask what is the
> > state of [1]?
>
> I'm not sure what [1] is? You mean Leons' ucm deletion patch?
>
> > [1] is not submitted as RFC, so I presume that is strong need to
> > drop this module.  And if we are dropping ucm module out of the
> > kernel than all the APIs which are only called by UCM module should
> > go out too.
>
> This is a good point, yes. Leon?

First let's drop that module.

>
> > And LAP message are part of it.  So like to know whether to invest
> > more energy in making this code right, which has almost no users?
>
> I wouldn't do anything related exclusively to ucm, yes.

Yes, we are not testing UCM.

>
> But.. this patch is still complicated, it is hard to see if all
> the readers are safe or not..

It was tested, no new bugs were discovered, so it is safe to assume that
Parav's series are not breaking IB and RoCE and safe for inclusion
It is targeted for ->next and we will have enough time to test it on
other fabrics.

Thanks

>
> Jason

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux