[openib-general] user_mad check question

[Rimmer, Todd]

Todd Rimmer
Chief Systems Architect              SilverStorm Technologies
Voice: 610-233-4852                   Fax: 610-233-4777
TRimmer at SilverStorm.com        www.SilverStorm.com
 

> From: Sean Hefty
> Sent: Friday, July 07, 2006 2:31 PM
> 
> The following check in user_mad is done when sending a MAD.
> 
> 	/*
> 	 * If userspace is generating a request that will generate a
> 	 * response, we need to make sure the high-order part of the
> 	 * transaction ID matches the agent being used to send the
> 	 * MAD.
> 	 */
> 	method = ((struct ib_mad_hdr *) packet->msg->mad)->method;
> 
> 	if (!(method & IB_MGMT_METHOD_RESP)       &&
> 	    method != IB_MGMT_METHOD_TRAP_REPRESS &&
> 	    method != IB_MGMT_METHOD_SEND) {
> 		tid = &((struct ib_mad_hdr *) packet->msg->mad)->tid;
> 		*tid = cpu_to_be64(((u64) agent->hi_tid) << 32 |
> 				   (be64_to_cpup(tid) & 0xffffffff));
> 	}
> 
> Does anyone know the reason for the last check (method != SEND)?
Isn't it
> still
> a requirement that the TID be unique?
> 
> I'm trying to add some validation checks against MADs sent from
userspace
> to
> protect against a client sending non-data RMPP MADs, duplicate MADs,
etc.,
> and
> the above check looks incorrect to me.
> 

While the TID can be appropriate for a SEND (it depends on management
class, some classes could chose to always use 0), this code fragment
cannot be sure if the SEND is a new request or a response to an existing
request.  Hence it cannot be certain if it should modify the TID.

I have not studied this whole code path, but it might hint at a problem
when handling MAD classes which use SEND instead of REQ/RESP type
messages.
Namely the hi_tid is not inserted for SEND.  Does other code on the
receive path expect hi_tid?

One such class which can use SEND to hold requests and their responses
is the BMA.  So can the SNMP tunnel and the CM (although CM is not
usually done from user space).  For the BMA case, a request can be:
Class=BM
Method=SEND
AttributeModifier has BM RESPONSE bit set


Todd Rimmer