On Thu, Aug 25, 2016 at 08:27:21AM -0400, Mikulas Patocka wrote: > On Wed, 24 Aug 2016, Masatake YAMATO wrote: > > > >> Are you talking about https://sourceforge.net/p/strace/mailman/message/34370586/ ? > > > > > > Yes. > > > > > >> The thread has apparently died without any follow-up from your side. > > > > > > I didn't receive that last message, it was probably sent only to the > > > mailing list address, not to my address. > > > > Can I expect you will submit updated version of the patch? > > > > Masatake YAMATO > > Here I'm sending the updated device mapper strace patch with the comments > addressed. (please send replies to my email address, I'm not on the strace > mailing list) > > Mikulas > > Index: strace/configure.ac > =================================================================== > --- strace.orig/configure.ac > +++ strace/configure.ac > @@ -363,6 +363,7 @@ AC_CHECK_HEADERS(m4_normalize([ > elf.h > inttypes.h > linux/bsg.h > + linux/dm-ioctl.h > linux/falloc.h > linux/fiemap.h > linux/filter.h > Index: strace/dm.c > =================================================================== > --- /dev/null > +++ strace/dm.c > @@ -0,0 +1,351 @@ > +#include "defs.h" > + > +#ifdef HAVE_LINUX_DM_IOCTL_H > + > +#include <sys/ioctl.h> > +#include <linux/dm-ioctl.h> > + > +static void > +dm_decode_device(const unsigned int code, const struct dm_ioctl *ioc) > +{ > + switch (code) { > + case DM_REMOVE_ALL: > + case DM_LIST_DEVICES: > + case DM_LIST_VERSIONS: > + break; > + default: > + if (ioc->dev) > + tprintf(", dev=makedev(%u, %u)", > + major(ioc->dev), minor(ioc->dev)); > + if (ioc->name[0]) { > + tprints(", name="); > + print_quoted_string(ioc->name, DM_NAME_LEN, > + QUOTE_0_TERMINATED); > + } > + if (ioc->uuid[0]) { > + tprints(", uuid="); > + print_quoted_string(ioc->uuid, DM_UUID_LEN, > + QUOTE_0_TERMINATED); > + } > + break; > + } > +} > + > +static void > +dm_decode_values(struct tcb *tcp, const unsigned int code, > + const struct dm_ioctl *ioc) > +{ > + if (entering(tcp)) { > + switch (code) { > + case DM_TABLE_LOAD: > + tprintf(", target_count=%"PRIu32"", > + ioc->target_count); > + break; > + case DM_DEV_SUSPEND: > + if (ioc->flags & DM_SUSPEND_FLAG) > + break; > + case DM_DEV_RENAME: > + case DM_DEV_REMOVE: > + case DM_DEV_WAIT: > + tprintf(", event_nr=%"PRIu32"", > + ioc->event_nr); > + break; > + } > + } else if (!syserror(tcp)) { > + switch (code) { > + case DM_DEV_CREATE: > + case DM_DEV_RENAME: > + case DM_DEV_SUSPEND: > + case DM_DEV_STATUS: > + case DM_DEV_WAIT: > + case DM_TABLE_LOAD: > + case DM_TABLE_CLEAR: > + case DM_TABLE_DEPS: > + case DM_TABLE_STATUS: > + case DM_TARGET_MSG: > + tprintf(", target_count=%"PRIu32"", > + ioc->target_count); > + tprintf(", open_count=%"PRIu32"", > + ioc->open_count); > + tprintf(", event_nr=%"PRIu32"", > + ioc->event_nr); > + break; > + } > + } > +} > + > +#include "xlat/dm_flags.h" > + > +static void > +dm_decode_flags(const struct dm_ioctl *ioc) > +{ > + tprints(", flags="); > + printflags(dm_flags, ioc->flags, "DM_???"); > +} > + > +static void > +dm_decode_dm_target_spec(struct tcb *tcp, const struct dm_ioctl *ioc, > + const char *extra, uint32_t extra_size) > +{ > + uint32_t i; > + uint32_t offset = ioc->data_start; > + for (i = 0; i < ioc->target_count; i++) { > + if (offset + (uint32_t)sizeof(struct dm_target_spec) >= offset && > + offset + (uint32_t)sizeof(struct dm_target_spec) < extra_size) { > + const struct dm_target_spec *s = > + (const struct dm_target_spec *)(extra + offset); > + tprintf(", {sector_start=%"PRIu64", length=%"PRIu64"", > + (uint64_t)s->sector_start, (uint64_t)s->length); > + if (!entering(tcp)) > + tprintf(", status=%"PRId32"", s->status); > + tprints(", target_type="); > + print_quoted_string(s->target_type, DM_MAX_TYPE_NAME, > + QUOTE_0_TERMINATED); > + tprints(", string="); > + print_quoted_string((const char *)(s + 1), extra_size - > + (offset + > + sizeof(struct dm_target_spec)), > + QUOTE_0_TERMINATED); > + tprintf("}"); > + if (entering(tcp)) > + offset = offset + s->next; > + else > + offset = ioc->data_start + s->next; This code trusts s->next; unfortunately, strace cannot trust syscall arguments, otherwise anything may happen, e.g. $ cat ioctl_dm.c #include <sys/ioctl.h> #include <linux/dm-ioctl.h> int main(void) { struct { struct dm_ioctl ioc; struct dm_target_spec spec; int i; } s = { .spec = { 0 }, .ioc = { .version[0] = DM_VERSION_MAJOR, .data_size = sizeof(s), .data_start = sizeof(s.ioc), .target_count = -1U } }; return !ioctl(-1, DM_TABLE_LOAD, &s); } $ strace -veioctl ./ioctl_dm btw, this parser lacks tests. How one can easily verify that it works correctly? For example how a test for strace ioctl parser may look like see tests/ioctl_* files. -- ldv
Attachment:
pgpfXQKJyNmR0.pgp
Description: PGP signature
-- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
