[PATCH] Add decoding for evdev ioctls.
Dmitry V. Levin
ldv at altlinux.org
Sat Jan 31 19:00:36 UTC 2015
On Fri, Jan 30, 2015 at 11:57:24AM +0100, Etienne Gemsa wrote:
> This patch introduces decoding for evdev ioctls. For EVIOCSFF the five first
> members are decoded, the union is not.
> The code was compiled and tested on x86_64 with Linux 3.13.
Thanks, see my comments below.
[...]
> diff --git a/evdev.c b/evdev.c
> new file mode 100644
> index 0000000..8597178
> --- /dev/null
> +++ b/evdev.c
> @@ -0,0 +1,347 @@
[...]
> +
> +#include <linux/input.h>
> +#include <stdlib.h>
> +#include "defs.h"
"defs.h" must be the first included header: it includes "config.h",
which in turn affects all the rest.
"<stdlib.h>" is already included by "defs.h".
ioctl.c guards inclusion of <linux/input.h> with HAVE_LINUX_INPUT_H.
> +#include "xlat/evdev_keycode.h"
> +#include "xlat/evdev_prop.h"
> +#include "xlat/evdev_leds.h"
> +#include "xlat/evdev_mtslots.h"
> +#include "xlat/evdev_events.h"
> +#include "xlat/evdev_switch.h"
> +#include "xlat/evdev_snd.h"
> +
> +void evdev_ioctl_decode_envelope(struct ff_envelope envelope)
Wouldn't it be better to pass the structure by reference?
> +{
> + tprintf(", envelope={attack_length=%" PRIu16 ", attack_level=%" PRIu16
> + ", fade_length=%" PRIu16 ", fade_level=%" PRIx32 "}",
> + envelope.attack_length, envelope.attack_level,
> + envelope.fade_length, envelope.fade_level);
This indentation lacks readability.
> +}
> +
> +int evdev_ff_effect_ioctl(struct tcb *tcp, const unsigned int code, long arg)
"code" is unused parameter in this function.
> +{
> + struct ff_effect ffe;
> +
> + if (!verbose(tcp) || umove(tcp, arg, &ffe) < 0)
> + return 0;
> +
> + tprintf(", {type=%" PRIu16 ", id=%" PRIu16 ", direction=%" PRIu16,
> + ffe.type, ffe.id, ffe.direction);
ffe.type is a good candidate for decoding with printxval.
> +
> + if (!abbrev(tcp)) {
> + tprintf(", trigger={button=%" PRIu16 ", interval=%" PRIu16 "}",
> + ffe.trigger.button, ffe.trigger.interval);
> + tprintf(", replay={lenght=%" PRIu16 ", delay=%" PRIu16 "}",
> + ffe.replay.length, ffe.replay.delay);
> + switch (ffe.type) {
> + case FF_CONSTANT:
> + tprintf(", constant_ef={%" PRIi16,
> + ffe.u.constant.level);
> + evdev_ioctl_decode_envelope(ffe.u.constant.envelope);
> + tprints("}");
> + return 1;
> + case FF_RAMP:
> + tprintf(", ramp={start_level=%" PRIi16
> + ", end_level=%" PRIi16,
> + ffe.u.ramp.start_level,
> + ffe.u.ramp.end_level);
> + evdev_ioctl_decode_envelope(ffe.u.ramp.envelope);
> + tprints("}");
> + return 1;
> + case FF_PERIODIC:
> + tprintf(", periodic_ef={waveform=%" PRIu16
> + ", period=%" PRIu16
> + ", magnitude=%" PRIi16
> + ", offset=%" PRIi16
> + ", phase=%" PRIu16,
> + ffe.u.periodic.waveform,
> + ffe.u.periodic.period,
> + ffe.u.periodic.magnitude,
> + ffe.u.periodic.offset,
> + ffe.u.periodic.phase);
> + evdev_ioctl_decode_envelope(ffe.u.periodic.envelope);
> + tprintf(", custom_len=%" PRIu32
> + ", *custom_data=%#lx}",
> + ffe.u.periodic.custom_len,
> + (unsigned long)ffe.u.periodic.custom_data);
> + return 1;
> + case FF_RUMBLE:
> + tprintf(", rumble={strong_magnitude=%" PRIu16
> + ", weak_magnitude=%" PRIu16 "}",
> + ffe.u.rumble.strong_magnitude,
> + ffe.u.rumble.weak_magnitude);
> + return 1;
> + case FF_SPRING:
> + case FF_FRICTION:
> + case FF_DAMPER:
> + case FF_INERTIA:
> + case FF_CUSTOM:
> + tprints(", ...}");
> + return 1;
> + default :
> + return 0;
At this point, it's too late to return 0.
> + }
> +
> + } else
> + tprints(", ...}");
> + return 1;
> +}
> +
> +struct input_mt_request_layout {
> + __u32 code;
> + __s32 values[];
> +};
Note that this defines a structure of variable size, which is of little
use in strace.
Also, we prefer types defined by <stdint.h>.
> +int evdev_abs_ioctl(struct tcb *tcp, const unsigned int code, long arg)
"code" is unused parameter in this function.
> +{
> + struct input_absinfo absinfo;
> +
> + if(tcp->u_rval == -1)
> + return 0;
The right way to check whether syscall has failed or not is
if (syserror(tcp))
The check itself is applicable only in case of exiting(tcp).
As this parser could be used in case of entering(tcp), e.g.
for parsing EVIOCSABS, this check should be made outside.
The alternative is to connect both checks together, e.g.:
if (exiting(tcp) && syserror(tcp))
return 0;
> + if (!verbose(tcp) || umove(tcp, arg, &absinfo) < 0)
> + return 0;
> +
> + tprintf(", {value=%" PRIu32 ", minimum=%" PRIu32,
> + absinfo.value, absinfo.minimum);
> + if (!abbrev(tcp)) {
> + tprintf(", maximum=%" PRIu32 ", fuzz=%" PRIu32,
> + absinfo.maximum, absinfo.fuzz);
> + tprintf(", flat=%" PRIu32 ", resolution=%" PRIu32,
> + absinfo.flat, absinfo.resolution);
> + tprints("}");
> + } else {
> + tprints(", ...}");
> + }
> + return 1;
> +}
> +
> +int evdev_keycode_ioctl(struct tcb *tcp, const unsigned int code, long arg)
> +{
> + struct input_keymap_entry ike;
> + unsigned int keycode[2];
> + unsigned i;
> +
> + if (tcp->u_rval == -1)
> + return 0;
> + if ((_IOC_SIZE(code)) == sizeof(unsigned int[2])) {
This should rather be checked earlier, e.g.
switch (code) {
case EVIOCGKEYCODE:
case EVIOCSKEYCODE:
return evdev_keycode(tcp, arg);
case EVIOCGKEYCODE_V2:
case EVIOCSKEYCODE_V2:
return evdev_keycode_v2(tcp, arg);
}
> + if (!verbose(tcp) || umove(tcp, arg, &keycode) < 0)
> + return 0;
> +
> + tprintf(", [%u, %s]", keycode[0],
> + xlookup(evdev_keycode, keycode[1]));
There is a function called printxval that has to be used instead of
xlookup in cases like this.
> + } else {
> + if (!verbose(tcp) || umove(tcp, arg, &ike) < 0)
> + return 0;
> +
> + tprintf(", {flags=%" PRIu8 ", len=%" PRIu8,
> + ike.flags, ike.len);
> + if (!abbrev(tcp)) {
> + tprintf(", index=%" PRIu16 ", keycode=%" PRIu32
> + ,ike.index, ike.keycode);
> + tprints(", scancode={");
> + for (i = 0; i < 32; i++)
> + tprintf(" %" PRIu8, ike.scancode[i]);
> + tprints("}}");
> + } else {
> + tprints(", ...}");
> + }
> + }
> + return 1;
> +}
> +
> +int evdev_decode_per_byte_ioctls(struct tcb *tcp, const unsigned code, long arg,
> + const struct xlat decode_flag[], const unsigned int max_flag)
"code" is unused parameter in this function.
> +{
> + if (tcp->u_rval <= 0)
> + return 0;
> +
> + char *decoded_arg = calloc(tcp->u_rval, sizeof(char));
> + if (decoded_arg == NULL) {
> + return 0;
> + }
> +
> + int passed;
> + int all_to_zero;
> + unsigned i;
> + unsigned j;
> + unsigned bit_offset;
> +
> + if (!verbose(tcp) || umovestr(tcp, arg, tcp->u_rval, decoded_arg) < 0) {
Shouldn't umoven be more appropriate here?
> + free(decoded_arg);
> + return 0;
> + }
> +
> + passed = 0;
> + all_to_zero = 1;
> + tprints(", [");
> + for (i = 0; i < tcp->u_rval; i++) {
> + if (decoded_arg[i]) {
> + for (j = 0; j < 8; j++) {
> + if (decoded_arg[i] & (1 << j)) {
> + bit_offset = i*8 + j;
> + tprintf("%s%s", (passed ? ", " : ""),
> + xlookup(decode_flag, bit_offset));
> + passed = 1;
> + }
> + }
> + all_to_zero = 0;
> + }
> + }
There is a function called next_set_bit that has to be used instead of
this double loop.
> +
> + if (all_to_zero)
> + tprints("0]");
> + else
> + tprints("]");
> +
> + free(decoded_arg);
> + return 1;
> +}
> +
> +int evdev_ioctl(struct tcb *tcp, const unsigned int code, long arg)
> +{
> + struct input_id id;
> + unsigned int rp_settings[2];
> + int i;
> + struct input_mt_request_layout mtslots;
> + int values_size;
> + int passed;
> +
> + if (entering(tcp))
> + return 0;
All that could be decoded on entering, should be decoded on entering.
All _IOC_WRITE ioctls could be decoded on entering. For example,
if (_IOC_DIR(code) == _IOC_WRITE) {
unsigned int val[2];
if (exiting(tcp))
return 1;
switch(code) {
case EVIOCSREP:
if (!verbose(tcp) ||
(umoven(tcp, arg, sizeof(val), (char *) val) < 0) {
tprintf(", %#lx", arg);
} else {
tprintf(", [delay=%u, period=%u]", val[0], val[1]);
}
return 1;
case EVIOCSKEYCODE:
evdev_keycode(tcp, arg);
return 1;
case EVIOCSKEYCODE_V2:
evdev_keycode_v2(tcp, arg);
return 1;
case EVIOCSFF:
evdev_ff_effect(tcp, arg);
return 1;
case EVIOCRMFF:
case EVIOCGRAB:
case EVIOCREVOKE:
case EVIOCSCLOCKID:
tprints(", ");
printnum_int(tcp, arg, "%u");
return 1;
}
if ((_IOC_NR(code) & ~ABS_MAX) == _IOC_NR(EVIOCSABS(0))) {
evdev_abs(tcp, code, arg);
return 1;
}
tprintf(", %#lx", arg);
return 1;
}
> + if (_IOC_NR(code) >= _IOC_NR(EVIOCGBIT(0,0))
> + && _IOC_NR(code) <= _IOC_NR(EVIOCGBIT(EV_MAX,0))) {
_IOC_NR(code) returns only 8 bits of 32-bit code.
Another 8 bits are filtered by _IOC_TYPE(code) switch in ioctl_decode.
The remaining _IOC_DIR(code) shouldn't be missed:
if (_IOC_DIR(code) == _IOC_READ &&
(_IOC_NR(code) & ~EV_MAX) == _IOC_NR(EVIOCGBIT(0, 0))) {
> + if(tcp->u_rval == -1)
> + return 0;
if (entering(tcp) || syserror(tcp))
return 0;
> + tprintf(", %s = [", xlookup(evdev_events, _IOC_NR(code) - 0x20));
There is a function called printxval that has to be used instead of
xlookup in cases like this.
> + printstr(tcp, arg, _IOC_SIZE(code));
EVIOCGBIT returns a bitmask, it has to be fetched with umoven and decoded
as other bitmasks.
> + tprintf("]");
> + return 1;
> + }
> +
> + if (_IOC_NR(code) >= _IOC_NR(EVIOCGABS(0))
> + && _IOC_NR(code) <= _IOC_NR(EVIOCGABS(ABS_MAX)))
> + return evdev_abs_ioctl(tcp, code, arg);
> +
> + if (_IOC_NR(code) >= _IOC_NR(EVIOCSABS(0))
> + && _IOC_NR(code) <= _IOC_NR(EVIOCSABS(ABS_MAX)))
> + return evdev_abs_ioctl(tcp, code, arg);
> +
> + switch (_IOC_NR(code)) {
_IOC_DIR and _IOC_SIZE (when appropriate) also have to be checked.
--
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20150131/1aa38bd3/attachment.bin>
More information about the Strace-devel
mailing list