[openib-general] [PATCH] user_mad: Fix length of user buffer copied when sending RMPP

[Hal Rosenstock]

On Mon, 2005-09-19 at 13:05, Roland Dreier wrote:
>     Hal> I already checked it in. There was an earlier change which
>     Hal> was just inteneded to change some formatting but I made a
>     Hal> mistake and made part of this change there where I
>     Hal> (mistakenly) also eliminated the subtraction of rmpp_hdr_size
>     Hal> you cite below.
> 
> I see... I hadn't done svn up.  I still think the change has to be
> wrong, though.  With your latest code:
> 
> 		/* Now, copy rest of message from user into send buffer */
> 		if (copy_from_user(((struct ib_rmpp_mad *) packet->msg->mad)->data,
> 				   buf + sizeof (struct ib_user_mad) + rmpp_hdr_size,
> 				   length + class_hdr_len)) {
> 
> At the beginning of the function,
> 
> 	length = count - sizeof (struct ib_user_mad);
> 
> We know class_hdr_len >= 0.  So that copy is copying
> 
> 	count - sizeof (struct ib_user_mad) + class_hdr_len
> 
> bytes from buf, at an offset of
> 
> 	sizeof (struct ib_user_mad) + rmpp_hdr_size
> 
> into the userspace buffer.  So it copies up to an offset of
> 
> 	count + class_hdr_len + rmpp_hdr_size
> 
> in buf.  But userspace only did a write of count bytes, so we're
> reading past the end of the userspace buffer.
> 
> What am I missing?

You are right that it is going past the end of the buffer :-( It does
seem to work but it appears that is just luck... I will fix it hopefully
correctly this time.

>     Hal> The length passed in for RMPP MADs is a little funny. In
>     Hal> osm_vendor_ibumad.c::osm_vendor_send for RMPP, the length of
>     Hal> the SA MAD header is subtracted off (but this includes the
>     Hal> MAD header, the RMPP header, and the SA class header). Even
>     Hal> if that length were to be made "more correct", it would only
>     Hal> include 1 RMPP header's worth as that is what in the buffer
>     Hal> being transmitted. That approach would require some slightly
>     Hal> different changes to user_mad to make the proper adjustments.
>     Hal> Would that approach be better ?
> 
> I don't really understand this either.  Doesn't userspace just pass in
> the data that the kernel passes on to ib_post_send_mad()?

It's not so direct (to ib_post_send_mad) for RMPP MADs.

-- Hal