On Sat, Jun 18, 2016 at 07:13:54PM +0200, Antoine Damhet wrote:
[...]
> +static int
> +decode_binder_commands_buffer(struct tcb *tcp, uintptr_t buffer, size_t pos,
> + size_t size, const struct xlat *x, const char *str)
> +{
> + if (size < sizeof(uint32_t)) {
> + tprints("[]");
> + return 0;
> + }
Wouldn't it be better to print the address of buffer instead?
> +
> + if (abbrev(tcp)) {
> + tprints("[...]");
> + return 0;
> + }
> +
> + uint32_t type;
> + tprints("[");
> +
> + goto print_one_commands_buffer;
> + while (pos + sizeof(uint32_t) <= size) {
Why the check is skipped for the first time?
> + tprints(", ");
> +
> +print_one_commands_buffer:
> + if (umove(tcp, buffer + pos, &type))
What's going to be fetched when buffer + pos overflows?
> + return 1;
What's going to be printed when this umove call fails?
> + if (_IOC_SIZE(type) > 0) {
> + tprints("[");
> + printxval(x, type, str);
> + tprints(", ");
> + if (pos + sizeof(type) + _IOC_SIZE(type) <= size)
> + printstr(tcp, buffer + pos + sizeof(type),
> + _IOC_SIZE(type));
> + tprints("]");
What's going to be printed when this condition is false?
> + } else
> + printxval(x, type, str);
> + pos += sizeof(uint32_t) + _IOC_SIZE(type);
What's going to happen when pos overflows?
> + }
> +
> + tprints("]");
> + return 0;
> +}
> +
> +static int
> +decode_binder_write_read(struct tcb *tcp, const long addr)
> +{
> + struct binder_write_read wr;
> +
> + if (entering(tcp)) {
> + tprints(", ");
> + if (umove_or_printaddr(tcp, addr, &wr))
> + return RVAL_DECODED | 1;
> +
> + tprintf("{write_size=%" PRIu64 ", write_consumed=%" PRIu64
> + ", write_buffer=",
> + (uint64_t)wr.write_size,
> + (uint64_t)wr.write_consumed);
> + if (decode_binder_commands_buffer(tcp, wr.write_buffer,
> + wr.write_consumed, wr.write_size,
> + binder_driver_commands, "BC_???")) {
Does the kernel take write_consumed into account?
If not, shouldn't 0 be passed to decode_binder_commands_buffer instead,
and what's the use of printing write_consumed on entering?
> + tprints("}");
> + return RVAL_DECODED | 1;
> + }
> +
> + tprintf(", read_size=%" PRIu64 ", read_consumed=%" PRIu64 "}",
> + (uint64_t)wr.read_size,
> + (uint64_t)wr.read_consumed);
If read_consumed is write only, what's the use of printing it on entering?
> + return 0;
> + }
> +
> + if (syserror(tcp) || umove(tcp, addr, &wr))
> + return RVAL_DECODED | 1;
> +
> + tprints(" => ");
> +
> + tprintf("{write_size=%" PRIu64 ", write_consumed=%" PRIu64
> + ", read_size=%" PRIu64 ", read_consumed=%" PRIu64
> + ", read_buffer=",
> + (uint64_t)wr.write_size,
> + (uint64_t)wr.write_consumed,
> + (uint64_t)wr.read_size,
> + (uint64_t)wr.read_consumed);
If write_size and read_size are read only, what's the use of printing them
on exiting?
[...]
> --- a/configure.ac
> +++ b/configure.ac
> @@ -440,6 +440,24 @@ AC_CHECK_HEADERS([linux/bpf.h], [
> fi
> ])
>
> +AC_CHECK_HEADERS([linux/android/binder.h], [[ #include <sys/types.h>
> +#include <linux/android/binder.h>])
> + AC_CHECK_DECLS(m4_normalize([BC_TRANSACTION, BC_REPLY,
> BC_ACQUIRE_RESULT,
> + BC_FREE_BUFFER, BC_INCREFS, BC_ACQUIRE,
> + BC_RELEASE, BC_DECREFS, BC_INCREFS_DONE,
> + BC_ACQUIRE_DONE, BC_ATTEMPT_ACQUIRE,
> + BC_REGISTER_LOOPER, BC_ENTER_LOOPER,
> + BC_EXIT_LOOPER, BC_REQUEST_DEATH_NOTIFICATION,
> + BC_CLEAR_DEATH_NOTIFICATION,
> BC_DEAD_BINDER_DONE,
> + BR_ERROR, BR_OK, BR_TRANSACTION, BR_REPLY,
> + BR_ACQUIRE_RESULT, BR_DEAD_REPLY,
> + BR_TRANSACTION_COMPLETE, BR_INCREFS, BR_ACQUIRE,
> + BR_RELEASE, BR_DECREFS, BR_ATTEMPT_ACQUIRE,
> + BR_NOOP, BR_SPAWN_LOOPER, BR_FINISHED,
> + BR_DEAD_BINDER,
> BR_CLEAR_DEATH_NOTIFICATION_DONE,
> + BR_FAILED_REPLY]),,,[ #include <sys/types.h>
Please list these constants sorted in one column like in other places.
> +#include <linux/android/binder.h>])])
> +
I'm not quite sure what's expected from defined(__ANDROID__) in binder.c
and whether this check is consistent with conditions where
<linux/android/binder.h> is not available but defined(__ANDROID__) is true.
--
ldv
pgpzVeLOcAnT8.pgp
Description: PGP signature
------------------------------------------------------------------------------ What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic patterns at an interface-level. Reveals which users, apps, and protocols are consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports. http://sdm.link/zohomanageengine
_______________________________________________ Strace-devel mailing list Strace-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/strace-devel
