Hi,
Le mercredi 17 décembre 2014 à 08:54 +0200, Haggai Eran a écrit :
> On 16/12/2014 14:33, Yann Droneaud wrote:
> > Le jeudi 11 décembre 2014 à 17:04 +0200, Haggai Eran a écrit :
> >> static inline int ib_copy_to_udata(struct ib_udata *udata, void *src,
> >> size_t len)
> >> {
> >> - return copy_to_user(udata->outbuf, src, len) ? -EFAULT : 0;
> >> + size_t copy_sz;
> >> +
> >> + copy_sz = min_t(size_t, len, udata->outlen);
> >> + return copy_to_user(udata->outbuf, src, copy_sz) ? -EFAULT : 0;
> >> }
> >
> >
> > This is not the place to do this: as I'm guessing the purpose of this
> > change from the patch in '[PATCH v3 07/17] IB/core: Add flags for on
> > demand paging support', you're trying to handle uverbs call from
> > a userspace program using a previous, shorter ABI.
>
> Yes, that was my intention.
>
> >
> > But that's hidding bug where userspace will get it wrong at passing the
> > correct buffer / size for all others uverb calls.
> >
> > That cannot work that way.
> >
> > In a previous patchset [1], I've suggested to add a check in
> > ib_copy_{from,to}_udata()[2][3] in order to check the input/output
> > buffer size to not read/write past userspace provided buffer
> > boundaries: in case of mismatch an error would be returned to
> > userspace.
> >
> > With the suggested change here, buffer overflow won't happen,
> > but the error is silently ignored, allowing uverb to return a
> > partial result, which is likely not expected by userspace as
> > it's a bit difficult to handle it gracefully.
> >
> > So this has to be removed, and a check on userspace response
> > buffer must be added to ib_uverbs_ex_query_device() instead.
>
> I agree that we shouldn't silently ignore bugs in userspace, but I'm not
> sure the alternative is maintainable. If we have in the future N new
> extensions to this verb, will we need to validate the user space given
> output buffer is one of the N possible sizes?
>
Yes.
Additionnaly the size should be checked related to the flags set in the
"comp_mask": eg. requiring IB_USER_VERBS_EX_QUERY_DEVICE_ODP but not
providing the expected response buffer should be an error.
Regards.
--
Yann Droneaud
OPTEYA
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
