On Mon, Jul 06, 2015 at 04:40:24PM +0200, Gabriel Laskar wrote:
> On Mon, 6 Jul 2015 12:35:52 +0200, Patrik Jakobsson wrote:
> > 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.
> 
> umove() can fail in multiple ways. For example, if the memory is not
> valid in the tracee, umove() will fail.

Yes, this is the most likely cause for umove() to fail,
and the most easily reproducible one, e.g.
ioctl(-1, DRM_IOCTL_VERSION, 42);

> Anyway, SYS_FUNC(ioctl) is a bit complicated, and the handling of the
> fallbacks on failure should be more generic.

What would be useful is a way for "on entering" parsers to return
"done with decoding" information to their callers.

This could be implemented by or'ing return value in the current semantics
with a flag with "done with decoding" meaning, e.g. RVAL_DONE.

If an ioctl parser returned RVAL_DONE, this would tell SYS_FUNC(ioctl)
that the decoding is finished but fallback decoding is needed, while
RVAL_DONE+1 would mean that the decoding is finished and no fallback
decoding is needed.


-- 
ldv

Attachment: pgpvjvUdXv3sO.pgp
Description: PGP signature

------------------------------------------------------------------------------
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.
https://www.gigenetcloud.com/
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel

Reply via email to