[PATCH v3] netlink: adjust decode_nlmsgerr for extended ACK reporting

[JingPiao Chen]

On Wed, Jul 19, 2017 at 06:30:41AM +0300, Dmitry V. Levin wrote:
> On Wed, Jul 19, 2017 at 11:07:19AM +0800, JingPiao Chen wrote:
> > Extended ACK reporting introduced by linux kernel commit
> > v4.11-rc5-1382-g2d4bc93.
> >
> > * netlink.h (NLM_F_CAPPED): New macro.
> > * netlink.c (decode_payload): Pass
> > nlmsghdr->nlmsg_flags & NLM_F_CAPPED to decode_nlmsgerr.
> > (decode_nlmsgerr): Adjust the length pass to
> > decode_nlmsghdr_with_payload.
> > ---
> >  netlink.c | 14 +++++++++++---
> >  netlink.h |  4 ++++
> >  2 files changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/netlink.c b/netlink.c
> > index d3ad8b0..122422b 100644
> > --- a/netlink.c
> > +++ b/netlink.c
> > @@ -297,7 +297,8 @@ decode_nlmsgerr(struct tcb *const tcp,
> >   const int fd,
> >   const int family,
> >   kernel_ulong_t addr,
> > - kernel_ulong_t len)
> > + kernel_ulong_t len,
> > + const bool capped)
> >  {
> >   struct nlmsgerr err;
> >
> > @@ -320,10 +321,16 @@ decode_nlmsgerr(struct tcb *const tcp,
> >   len -= offsetof(struct nlmsgerr, msg);
> >
> >   if (len) {
> > + unsigned int payload = len;
> > +
> >   tprints(", msg=");
> >   if (fetch_nlmsghdr(tcp, &err.msg, addr, len)) {
> > + payload = capped ? sizeof(err.msg) : err.msg.nlmsg_len;
>
> Subsequent decode_nlmsghdr_with_payload call already does all necessary
> checks, why do you bother with err.msg.nlmsg_len here?

I need to get the length of nlmsgerr attribute ([PATCH v2 3/4]).

net/netlink/af_netlink.c:
void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
const struct netlink_ext_ack *extack)
{
...
struct nlmsgerr *errmsg;
size_t payload = sizeof(*errmsg);
...
if (err) {
if (!(nlk->flags & NETLINK_F_CAP_ACK))
payload += nlmsg_len(nlh);
else
flags |= NLM_F_CAPPED;
...
} else {
flags |= NLM_F_CAPPED;
...
}
...
memcpy(&errmsg->msg, nlh, payload > sizeof(*errmsg) ? nlh->nlmsg_len :
sizeof(*nlh));
...
}

the length of payload is sizeof(struct nlmsghdr) or err.msg.nlmsg_len.

>
> > + if (payload > len)
> > + payload = len;
>
> Consider this instead:
>
> const unsigned int payload =
> (capped && sizeof(err.msg) < len) ? sizeof(err.msg) : len;
>

You means:

diff --git a/netlink.c b/netlink.c
index d3ad8b0..cd6f40c 100644
--- a/netlink.c
+++ b/netlink.c
@@ -322,8 +323,11 @@ decode_nlmsgerr(struct tcb *const tcp,
  if (len) {
  tprints(", msg=");
  if (fetch_nlmsghdr(tcp, &err.msg, addr, len)) {
+ const unsigned int payload =
+ (capped && sizeof(err.msg) < len) ? sizeof(err.msg) : len;
+
  decode_nlmsghdr_with_payload(tcp, fd, family,
-     &err.msg, addr, len);
+     &err.msg, addr, payload);
  }
  }

Can not get the attribute length.

> > +
> >   decode_nlmsghdr_with_payload(tcp, fd, family,
> > -     &err.msg, addr, len);
> > +     &err.msg, addr, payload);
>

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