[PATCH] tests: check decoding of NETLINK_SOCK_DIAG AF_UNIX messages

[JingPiao Chen]

2017-06-14 12:35 GMT+08:00 Dmitry V. Levin <ldv at altlinux.org>:
> On Wed, Jun 14, 2017 at 11:53:02AM +0800, JingPiao Chen wrote:
> > On Wed, Jun 14, 2017 at 05:06:06AM +0300, Dmitry V. Levin wrote:
> > > On Wed, Jun 14, 2017 at 03:19:46AM +0300, Dmitry V. Levin wrote:
> > > > On Wed, Jun 14, 2017 at 07:49:34AM +0800, JingPiao Chen wrote:
> > > > > On Tue, Jun 13, 2017 at 07:10:13PM +0300, Dmitry V. Levin wrote:
> > > > > > On Tue, Jun 13, 2017 at 07:04:21PM +0800, JingPiao Chen wrote:
> > > > > > > +static void
> > > > > > > +test_unix_diag_req(const int fd)
> > > > > > > +{
> > > > > > > ...
> > > > > > > + /* short read of unix_diag_req */
> > > > > > > + nlh = nlh0 - (sizeof(*req) - 1);
> > > > > > > + memmove(nlh, nlh0 - sizeof(*req), NLMSG_HDRLEN +
sizeof(*req) - 1);
> > > > > > > +
> > > > > > > + rc = sendto(fd, nlh, NLMSG_HDRLEN + sizeof(*req),
MSG_DONTWAIT,
> > > > > > > +    NULL, 0);
> > > > > > > +
> > > > > > > + printf("sendto(%d, {{len=%u, type=SOCK_DIAG_BY_FAMILY"
> > > > > > > +       ", flags=NLM_F_REQUEST, seq=0, pid=0},
{family=AF_UNIX, %p}}"
> > > > > > > +       ", %u, MSG_DONTWAIT, NULL, 0) = %s\n",
> > > > > > > +       fd, NLMSG_HDRLEN + (unsigned int) sizeof(*req),
> > > > > > > +       NLMSG_DATA(nlh) + 1,
> > > > > > > +       NLMSG_HDRLEN + (unsigned int) sizeof(*req),
> > > > > > > +       sprintrc(rc));
> > > > > > > +}
> > > > > > >
> > > > > > > When (sizeof(*req) - 1 - sizeof(*family) > DEFAULT_STRLEN),
the test
> > > > > will
> > > > > > > fail.
> > > > > >
> > > > > > Why?  In this part of the test strace is expected to print a
pointer,
> > > > > > not a string, so it shouldn't be affected by DEFAULT_STRLEN.
> > > > > This fault appear when call test_inet_diag_req.
> > > > >
> > > > > Call stack is decode_inet_diag_req -->> decode_family -->>
printstrn
> > > > > len = sizeof(*req) - 1 - sizeof(*family),
> > > > > When sizeof(*req) - 1 - sizeof(*family) > DEFAULT_STRLEN, umoven
will
> > > > > return true, because: (util.c: 770)
> > > > > size = max_strlen + 1;
> > > > > if (size > len)
> > > > > size = len;
> > > >
> > > > I see.  Would it be better to print the address instead of the
string
> > > > in case of umove failure, e.g.
> > > >
> > > > if (len < sizeof(msg))
> > > > return false;
> > > >
> > > > if (umove_or_printaddr(tcp, addr, &msg))
> > > > return true;
> > >
> > > OK, I've changed this parser a bit and pushed it again to ldv/netlink
> > > branch, please have a look.
> > Ok, thank you.
>
> ... and again, amending the last commit to simplify subsequent
> netlink_sock_diag test changes.

I have some questions, why do not use:
if (len < sizeof(msg))
return false;

if (umove_or_printaddr(tcp, addr, &msg))
return true;

Can we remove offset?

decode_unix_diag_msg(struct tcb *const tcp,
    const struct nlmsghdr *const nlmsghdr,
    const uint8_t family,
    const kernel_ulong_t addr,
    const kernel_ulong_t len)
{
struct unix_diag_msg msg;

tprints("{udiag_family=");
printxval(addrfams, family, "AF_???");

tprints(", ");
if (len >= sizeof(msg)) {
if (umove(tcp, addr, &msg) < 0)
printaddr(addr + sizeof(msg.udiag_family));
else {
tprints("udiag_type=");
printxval(socktypes, msg.udiag_type, "SOCK_???");
tprintf(", udiag_state=");
printxval(tcp_states, msg.udiag_state, "TCP_???");
tprintf(", udiag_ino=%" PRIu32
", udiag_cookie=[%" PRIu32 ", %" PRIu32 "]",
msg.udiag_ino,
msg.udiag_cookie[0], msg.udiag_cookie[1]);
}
} else
tprints("...");
tprints("}");
}

Can we print address when len < sizeof(msg)?
Why print ... when len < sizeof(msg)?

decode_unix_diag_msg(struct tcb *const tcp,
    const struct nlmsghdr *const nlmsghdr,
    const uint8_t family,
    const kernel_ulong_t addr,
    const kernel_ulong_t len)
{
struct unix_diag_msg msg;

tprints("{udiag_family=");
printxval(addrfams, family, "AF_???");

tprints(", ");
if (!umove(tcp, addr, &msg)) {
tprints("udiag_type=");
printxval(socktypes, msg.udiag_type, "SOCK_???");
tprintf(", udiag_state=");
printxval(tcp_states, msg.udiag_state, "TCP_???");
tprintf(", udiag_ino=%" PRIu32
", udiag_cookie=[%" PRIu32 ", %" PRIu32 "]",
msg.udiag_ino,
msg.udiag_cookie[0], msg.udiag_cookie[1]);
} else
printaddr(addr + sizeof(msg.udiag_family));
tprints("}");
}

--
JingPiao Chen
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20170614/15cc1ea8/attachment.html>