Re: [PATCHv4 for-3.13 00/10] create_flow/destroy_flow fixes for v3.13

[Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>]

Hi Or, Roland.

Le mardi 17 décembre 2013 à 12:35 +0200, Or Gerlitz a écrit :
> On Tue, Dec 17, 2013 at 11:58 AM, Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org> wrote:
> >
> > Please find the fourth revision of a patchset against create_flow/destroy_flow
> > and associated extended command scheme.
> 
> Yann, note that V3 is already applied, so you need to change incremental changes

Thanks for pointing this, Or.

Unfortunately, some patches in V4 were replacement for patches in V3.
In this particular case, I've rework the commit messages in V4, so these
changes might be lost.

In this other hand, V3 is not applied asis in for-next branch of
Roland's tree. In fact Roland seems to have dropped this patch:
[PATCHv3 for-3.13 2/9] IB/uverbs: remove implicit cast in INIT_UDATA()
http://marc.info/?i=a6da021658e9a1cee5cf9f6db61694711cc2852d.1386798254.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

This, then, require a conflict fix when applying next patch:
[PATCHv3 for-3.13 3/9] IB/uverbs: set outbuf to NULL when no core
response space is provided
http://marc.info/?i=d4ae6494fb2dc1f8a81e8d8e90d8084b37d15ba2.1386798254.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

And another for this later patch:
[PATCHv3 for-3.13 8/9] IB/uverbs: check access to userspace response
 buffer in extended command
http://marc.info/?i=701fc3ee1136bbb4a0fd3d560c3ec1ee3bb3b828.1386798254.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org

And I'm a bit disappointed about this particular patch applied with a
conflict fix:

https://git.kernel.org/cgit/linux/kernel/git/roland/infiniband.git/commit/?h=for-next&id=2f92baf8313756fd32da4d2a24abc67c8f4f7026

	if (!access_ok(VERIFY_WRITE,
		       (const void __user *) (unsigned long) ex_hdr.response,
		       (hdr.out_words + ex_hdr.provider_out_words) * 8))
		return -EFAULT;

https://git.kernel.org/cgit/linux/kernel/git/roland/infiniband.git/tree/drivers/infiniband/core/uverbs_main.c?h=for-next&id=2f92baf8313756fd32da4d2a24abc67c8f4f7026#n679

Checking for write access on a pointer to const doesn't sound right.
It's harmless, and, if sparse/coccinelle doesn't have a check for such,
it should be added to report a warning.

BTW, I would prefer being responsible of my own mistakes :)

I try to be helpful and open to fix my patchset so that they are easier
to merge. In this particular case, the fix to my commit was not
'agreed': I haven't the chance to review it before being commited since
it wasn't discussed by mail.

So I hope the infiniband/for-next branch could be rewrote (it's a branch
for linux-next, do people care of it being rebased ?) to include my new
commit messages and discard the not so incorrect fix to access_ok()
call. And perhaps include the patch adding a 'response' pointer as
'common pattern' of uverbs function.

Just tell me what you want me to do to ease the integration of my fixes.

Regards.

-- 
Yann Droneaud
OPTEYA


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html