[PATCH 2/5] Add decoding for parametered ioctls
Dmitry V. Levin
ldv at altlinux.org
Sat Jan 17 01:32:40 UTC 2015
On Thu, Jan 15, 2015 at 08:32:27PM +0100, Gabriel Laskar wrote:
[...]
> --- a/defs.h
> +++ b/defs.h
> @@ -733,6 +733,7 @@ extern void print_loff_t(struct tcb *, long);
> extern const struct_ioctlent *ioctl_lookup(unsigned long);
> extern const struct_ioctlent *ioctl_next_match(const struct_ioctlent *);
> extern int ioctl_decode(struct tcb *, long, long);
> +extern int ioctl_decode_number(unsigned long);
ioctl command number has a 32-bit unsigned integer type; here is how it's
defined in the kernel:
fs/ioctl.c:SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd, unsigned long, arg)
If the kernel completely ignores other bits on 64-bit architectures,
why should strace care?
Let's follow the kernel and treat it as unsigned int.
Maybe a longer function name like ioctl_decode_command or
ioctl_decode_command_number would be easier to read.
> --- a/io.c
> +++ b/io.c
> @@ -398,13 +398,18 @@ sys_ioctl(struct tcb *tcp)
> if (entering(tcp)) {
> printfd(tcp, tcp->u_arg[0]);
> tprints(", ");
> - iop = ioctl_lookup(tcp->u_arg[1]);
> - if (iop) {
> - tprints(iop->symbol);
> - while ((iop = ioctl_next_match(iop)))
> - tprintf(" or %s", iop->symbol);
> - } else
> - tprintf("%#lx", tcp->u_arg[1]);
> +
> + int ret = ioctl_decode_number(tcp->u_arg[1]);
> + if (!ret) {
If the only use of this new variable is to assign it once and test it
right after assignment, why should we need the variable at all?
> --- a/ioctl.c
> +++ b/ioctl.c
> @@ -70,6 +70,121 @@ ioctl_next_match(const struct_ioctlent *iop)
> }
>
> int
> +evdev_decode_number(unsigned long arg)
> +{
Since this new function is not going to be used outside the file where it's
defined, it has to be static.
> + unsigned long nr = _IOC_NR(arg);
> +
> + if (nr >= 0x20 && nr <= 0x20 + 0x1f) {
> + tprintf("EVIOCGBIT(ev=%lu, len=%lu)", nr - 0x20, _IOC_SIZE(arg));
strace usually doesn't print symbolic names of arguments passed to
functions.
Do we need to print symbolic names of EVIOC*'s arguments?
All ioctl commands are either _IOC_NONE, _IOC_READ, _IOC_WRITE, or
_IOC_READ|_IOC_WRITE. All EVIOC* ioctl commands are strictly _IOC_READ or
_IOC_WRITE. It means that _IOC_DIR(arg) also has to be tested.
> + return 1;
> + } else if (nr >= 0x40 && nr <= 0x40 + 0x3f) {
> + tprintf("EVIOCGABS(abs=%lu)", nr - 0x40);
> + return 1;
> + } else if (nr >= 0xc0 && nr <= 0xc0 + 0x3f) {
> + tprintf("EVIOCSABS(abs=%lu)", nr - 0xc0);
> + return 1;
> + }
> +
> + switch (_IOC_NR(nr)) {
> + case 0x06:
> + tprintf("EVIOCGNAME(len=%lu)", _IOC_SIZE(arg));
> + return 1;
> + case 0x07:
> + tprintf("EVIOCGPHYS(len=%lu)", _IOC_SIZE(arg));
> + return 1;
> + case 0x08:
> + tprintf("EVIOCGUNIQ(len=%lu)", _IOC_SIZE(arg));
> + return 1;
> + case 0x09:
> + tprintf("EVIOCGPROP(len=%lu)", _IOC_SIZE(arg));
> + return 1;
> + case 0x0a:
> + tprintf("EVIOCGMTSLOTS(len=%lu)", _IOC_SIZE(arg));
> + return 1;
> + case 0x18:
> + tprintf("EVIOCGKEY(len=%lu)", _IOC_SIZE(arg));
> + return 1;
> + case 0x19:
> + tprintf("EVIOCGLED(len=%lu)", _IOC_SIZE(arg));
> + return 1;
> + case 0x1a:
> + tprintf("EVIOCGSND(len=%lu)", _IOC_SIZE(arg));
> + return 1;
> + case 0x1b:
> + tprintf("EVIOCGSW(len=%lu)", _IOC_SIZE(arg));
> + return 1;
> + default:
> + return 0;
> + }
> +}
> +
> +int
> +hiddev_decode_number(unsigned long arg)
> +{
> + switch (_IOC_NR(arg)) {
> + case 0x04:
> + tprintf("HIDIOCGRAWNAME(len=%lu)", _IOC_SIZE(arg));
> + return 1;
> + case 0x05:
> + tprintf("HIDIOCGRAWPHYS(len=%lu)", _IOC_SIZE(arg));
> + return 1;
> + case 0x06:
> + if (_IOC_DIR(arg) == IOC_IN) {
_IOC_DIR(arg) returns a value that is already _IOC_DIRSHIFT'ed and
_IOC_DIRMASK'ed, so it's not appropriate to compare this value with
IOC_IN or IOC_OUT, _IOC_READ and _IOC_WRITE should be used instead.
> + tprintf("HIDIOCSFEATURE(len=%lu)", _IOC_SIZE(arg));
> + } else {
> + tprintf("HIDIOCGNAME(len=%lu)", _IOC_SIZE(arg));
> + }
HIDIOCSFEATURE is _IOC_WRITE|_IOC_READ, that is, both bits must be set.
HIDIOCGNAME is _IOC_READ only.
That is, if only _IOC_READ is set, it's HIDIOCGNAME.
If both _IOC_READ and _IOC_WRITE bits are set, it's HIDIOCSFEATURE.
Otherwise, it's something different.
> + return 1;
> + case 0x07:
> + tprintf("HIDIOCGFEATURE(len=%lu)", _IOC_SIZE(arg));
> + return 1;
> + case 0x12:
> + tprintf("HIDIOCGPHYS(len=%lu)", _IOC_SIZE(arg));
There are other 'H' 0x12 ioctl commands, but only HIDIOCGPHYS has
_IOC_DIR(arg) == _IOC_READ :
linux$ git grep -Fh "'H', 0x12" include/
#define HDA_IOCTL_GET_WCAP _IOWR('H', 0x12, struct hda_verb_ioctl)
#define HIDIOCGPHYS(len) _IOC(_IOC_READ, 'H', 0x12, len)
#define SNDRV_EMU10K1_IOCTL_CODE_PEEK _IOWR('H', 0x12, struct snd_emu10k1_fx8010_code)
#define SNDRV_SB_CSP_IOCTL_UNLOAD_CODE _IO('H', 0x12)
This is another example why _IOC_DIR(arg) check is necessary.
> + return 1;
> + default:
> + return 0;
> + }
> +}
> +
> +int
> +ioctl_decode_number(unsigned long arg)
> +{
> + switch (_IOC_TYPE(arg)) {
> + case 'E':
> + return evdev_decode_number(arg);
> + case 'H':
> + return hiddev_decode_number(arg);
> + case 'M':
> + if (_IOC_DIR(arg) == IOC_IN) {
> + tprintf("MIXER_WRITE(dev=%lu)", _IOC_NR(arg));
> + } else {
> + tprintf("MIXER_READ(dev=%lu)", _IOC_NR(arg));
> + }
First, MIXER_READ is _IOC_READ, and MIXER_WRITE is _IOC_WRITE|_IOC_READ.
Second, we cannot assume that any 'M' ioctl command is either MIXER_READ
or MIXER_WRITE.
Please have a look at mtd.c :)
> + return 1;
> + case 'U':
> + if (_IOC_NR(arg) == 0x2c) {
> + tprintf("UI_GET_SYSNAME(len=%lu)", _IOC_SIZE(arg));
> + return 1;
> + }
Luckily, we don't have collisions on 0x552c at this moment. However,
there are other 'U' ioctl commands, e.g. defined in <linux/usbdevice_fs.h>
and <sound/asound.h>, so some day this issue might arise.
> + return 0;
> + case 'j':
> + if (_IOC_NR(arg) == 0x13) {
> + tprintf("JSIOCGNAME(len=%lu)", _IOC_SIZE(arg));
> + return 1;
> + }
> + return 0;
> + case 'k':
> + if (_IOC_NR(arg) == 0) {
> + tprintf("SPI_IOC_MESSAGE(N=%lu)", _IOC_SIZE(arg));
> + return 1;
> + }
KYRO_IOCTL_OVERLAY_CREATE has the same type|number code 0x6b00 as
SPI_IOC_MESSAGE, but they have different dir codes. This is yet another
example why _IOC_DIR(arg) check is necessary.
--
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/20150117/a369b9aa/attachment.bin>
More information about the Strace-devel
mailing list