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

[Roland Dreier]

    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?

    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()?

 - R.