Re: [PATCH v3 4/5] drm: Add decoding of i915 ioctls

Patrik Jakobsson
2015-07-06T10:35:52Z

On Fri, Jul 03, 2015 at 03:36:09AM +0300, Dmitry V. Levin wrote:
> On Wed, Jul 01, 2015 at 02:52:47PM +0200, Patrik Jakobsson wrote:
> [...]
> > --- a/drm.c
> > +++ b/drm.c
> > @@ -35,6 +35,9 @@
> >
> > #define DRM_MAX_NAME_LEN 128
> >
> > +extern int drm_i915_decode_number(struct tcb *tcp, unsigned int arg);
>
> Please rename "arg" to "code", and ...
>
> > +extern int drm_i915_ioctl(struct tcb *tcp, const unsigned int code, long arg);
>
> ... move both declarations to defs.h to make them visible also
> in the file where these functions are defined.
>
> [...]
> > +static int i915_setparam(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > + struct drm_i915_setparam param;
> > +
> > + if (entering(tcp)) {
> > + if (umove(tcp, arg, &param))
> > + return 0;
> > +
> > + tprints(", {param=");
> > + printxval(drm_i915_setparams, param.param, "I915_PARAM_???");
> > + tprintf(", value=%d}", param.value);
> > + }
> > +
> > + return 1;
> > +}
>
> In this and most of other parsers of _IOC_WRITE ioctls added by this and
> the next patches, any error in parser that leads to "return 0" will result
> to disabled "arg" decoding, including the fallback decoding performed by
> sys_ioctl.
>
> Maybe it's time to deal with this issue in a more generic way.
>

Yes, I'm thinking SYS_FUNC(ioctl) could be improved. But on the other hand how
likely is it that we fail in umove and what chance do we have to recover from
that anyway? All I can think of is OOM.

>
> --
> ldv

------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.