Re: [PATCHv1 0/6] IB/uverbs: check request parameters

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

Hi,

Le lundi 04 mai 2015 à 21:56 +0200, Yann Droneaud a écrit :
> Le lundi 04 mai 2015 à 13:45 -0400, Doug Ledford a écrit :
> > On Mon, 2015-05-04 at 15:00 +0200, Yann Droneaud wrote:
> > > 
> > > Please find a patchset against uverbs to improve the checks done
> > > on uverbs request parameters. This patchset in an extract of a
> > > previous patchset sent some times ago[1].
> > 
> > I reviewed those patches last time they came around.  In general, I'm on
> > board with the idea of cleaning up option checking, but in your original
> > patch submission you point out that the patches have a non-0 chance of
> > breaking user applications that pass in parameters that wouldn't pass
> > these checks but work anyway.  Have you done any looking into that
> > possibility?
> > 
> 
> I believe applications using libibverbs are immune of any kind of
> regression.
> 
> But I cannot tell for sure for applications that access directly uverbs:
> those could be doing broken things.
> 
> Anyway, we should prevent such applications to broke into the kernel:
> integer over/under flow or buffer overflow are not acceptable.
> 

I would also add, that, being stricter doesn't introduce a new, 
diverging behavior that newer applications could rely on, so if one 
existing application is found to be broken by the updated checks, those 
patchs could simply be reverted without falling into a compatibility 
nightmare where we would have to support two different behaviors.

> > > I've provided some explanation of the issues partialy addressed
> > > by this patchset in a previous message[2].
> > > 
> > > As we're now addressing overflows, I think it's time to apply
> > > this patchset.
> > > 
> > > Changes since v0 [3]
> > > - updated against v4.1-rc2
> > > - incorporated patches to add check on response buffer using
> > >   access_ok()
> > > 
> > > [1] "[PATCH 00/22] infiniband: improve userspace input check"
> > > 
> > > http://marc.info/?i=cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
> > > http://mid.gmane.org/cover.1376847403.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
> > > 
> > > [2] "Re: [PATCHv4 for-3.13 00/10] create_flow/destroy_flow fixes for v3.13"
> > > 
> > > http://marc.info/?i=1387493822.11925.217.camel-bi+AKbBUZKY6gyzm1THtWQ@public.gmane.orgomain
> > > http://mid.gmane.org/1387493822.11925.217.camel-bi+AKbBUZKY6gyzm1THtWWGXanvQGlWp@public.gmane.orgmain
> > > 
> > > [3] "[PATCH 0/4] IB/uverbs: check request parameters"
> > > 
> > > http://marc.info/?i=cover.1405884453.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
> > > http://mid.gmane.org/cover.1405884453.git.ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org
> > > 
> > > Yann Droneaud (6):
> > >   IB/uverbs: check userspace input buffer size
> > >   IB/uverbs: check userspace output buffer size
> > >   IB/uverbs: check userspace output buffer size in ib_uverbs_poll_cq()
> > >   IB/uverbs: subtract command header from input size
> > >   IB/uverbs: move cast from u64 to void __user pointer to its own
> > >     variable
> > >   IB/uverbs: check access to userspace response buffer
> > > 
> > >  drivers/infiniband/core/uverbs_cmd.c         | 449 +++++++++++++++++++++------
> > >  drivers/infiniband/core/uverbs_main.c        |  29 +-
> > >  drivers/infiniband/hw/mlx5/cq.c              |   6 +-
> > >  drivers/infiniband/hw/mlx5/main.c            |   2 +-
> > >  drivers/infiniband/hw/mlx5/srq.c             |   6 +-
> > >  drivers/infiniband/hw/mthca/mthca_provider.c |   2 +-
> > >  6 files changed, 382 insertions(+), 112 deletions(-)
> > > 
> > 
> > 
> 

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