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

[Hal Rosenstock]

On Mon, 2005-09-19 at 12:33, Roland Dreier wrote:
> What version of user_mad.c is this against?  

I already checked it in. There was an earlier change which was just
inteneded to change some formatting but I made a mistake and made part
of this change there where I (mistakenly) also eliminated the
subtraction of rmpp_hdr_size you cite below.

> It doesn't apply to the
> latest subversion, since you have the chunk
> 
>  		if (copy_from_user(((struct ib_rmpp_mad *) packet->msg->mad)->data,
>  				   buf + sizeof (struct ib_user_mad) + rmpp_hdr_size,
> -				   length)) {
> +				   length + class_hdr_len)) {
> 
> but the current code looks like
> 
> 		if (copy_from_user(((struct ib_rmpp_mad *) packet->msg->mad)->data,
> 				   buf + sizeof (struct ib_user_mad) + rmpp_hdr_size,
> 				   length - rmpp_hdr_size)) {
> 
> I don't see how the current code could be wrong: at the beginning of
> the function, we do:
> 
> 	length = count - sizeof (struct ib_user_mad);
> 
> so length is the size of the buffer passed in by userspace, less the
> size of our user_mad header.  Then in the copy_from_user() call, we're
> copying from an offset of sizeof (struct ib_user_mad) + rmpp_hdr_size
> after the beginning of the buffer, so we should copy at most the size
> of the buffer less that offset, which is exactly length - rmpp_hdr_size.

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

> If I'm wrong, can you regenerate your patch against the current code
> and provide a better changelog entry that describes what you're fixing?

I can regenerate the diff of these 2 versions together if you want or
redo this again with the other approach which might be clearer. What do
you think ?

-- Hal