[PATCH] netlink: introduce nla_policy system to parse netlink attribute data
Dmitry V. Levin
ldv at altlinux.org
Fri Jun 16 21:35:15 UTC 2017
On Sat, May 20, 2017 at 07:42:19PM +0800, JingPiao Chen wrote:
> * defs.h (NLA_*): New enum.
> (nla_policy): New struct.
> (decode_nlattr): Adapt prototype.
> * netlink.c (decode_nlattr_data): New function.
> (decode_nlattr_with_data): Use decode_nlattr_data.
> * netlink_sock_diag.c (decode_inet_diag_req_compat)
> (decode_inet_diag_req_v2, decode_inet_diag_msg)
> (decode_netlink_diag_msg, (decode_packet_diag_msg)
> (decode_smc_diag_msg, decode_unix_diag_msg): Add
> policy_size, policy, fallback_parser and opaque_data arguments.
> All callers updated.
>
> Co-authored-by: Fabien Siron <fabien.siron at epita.fr>
> ---
> defs.h | 40 ++++++++++++-
> netlink.c | 161 +++++++++++++++++++++++++++++++++++++++++++++++++---
> netlink_sock_diag.c | 14 ++---
> 3 files changed, 198 insertions(+), 17 deletions(-)
>
> diff --git a/defs.h b/defs.h
> index 7e3912d..1ebb89e 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -630,11 +630,47 @@ extern void
> tprint_iov_upto(struct tcb *, kernel_ulong_t len, kernel_ulong_t addr,
> enum iov_decode, kernel_ulong_t data_size);
>
> +enum {
> + NLA_UNSPEC,
> + NLA_U8,
> + NLA_U16,
> + NLA_U32,
> + NLA_U64,
> + NLA_STRING,
> + NLA_FLAG,
> + NLA_MSECS,
> + NLA_NESTED,
> + NLA_NESTED_COMPAT,
> + NLA_NUL_STRING,
> + NLA_BINARY,
> + NLA_S8,
> + NLA_S16,
> + NLA_S32,
> + NLA_S64
> +};
I think this enum doesn't belong to defs.h; would you mind creating
a separate header file, e.g. nlattr.h, and placing all nlattr related
declaration there?
> +
> +struct nla_policy {
> + uint16_t type;
> + uint16_t len;
> +};
> +
> extern void
> decode_netlink(struct tcb *, int fd, kernel_ulong_t addr, kernel_ulong_t len);
> extern void
> -decode_nlattr(struct tcb *tcp, kernel_ulong_t addr, kernel_ulong_t len,
> - int, const struct xlat *, const char *);
> +decode_nlattr(struct tcb *tcp,
> + kernel_ulong_t addr,
> + kernel_ulong_t len,
> + int hdrlen,
> + const struct xlat *table,
> + const char *dflt,
> + unsigned int policy_size,
> + const struct nla_policy *policy,
It would be more logical if "policy_size" argument was specified after
"policy" argument.
> + bool (*fallback_parser)(struct tcb *,
> + kernel_ulong_t addr,
> + kernel_ulong_t len,
> + int nla_type,
> + void *opaque_data),
> + void *const opaque_data);
>
> extern void tprint_open_modes(unsigned int);
> extern const char *sprint_open_modes(unsigned int);
> diff --git a/netlink.c b/netlink.c
> index 52f3806..cadfccb 100644
> --- a/netlink.c
> +++ b/netlink.c
> @@ -78,9 +78,138 @@ print_nlattr(const struct nlattr *const nla,
> }
>
> static void
> -decode_nlattr_with_data(struct tcb *tcp, kernel_ulong_t addr,
> - kernel_ulong_t len, const struct xlat *table,
> - const char *dflt)
> +decode_nlattr_data(struct tcb *tcp,
> + kernel_ulong_t addr,
> + kernel_ulong_t len,
> + unsigned int nla_type,
> + unsigned int policy_size,
> + const struct nla_policy *policy,
> + bool(*fallback_parser)(struct tcb *,
> + kernel_ulong_t addr,
> + kernel_ulong_t len,
> + int nla_type,
> + void *opaque_data),
> + void *const opaque_data)
> +{
> + if (!policy || nla_type >= policy_size) {
> + printstrn(tcp, addr, len);
> + return;
> + }
> +
> + switch (policy[nla_type].type) {
> + case NLA_U8: {
> + uint8_t num;
> +
> + if (len < sizeof(num))
> + printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);
> + else if (!umove_or_printaddr(tcp, addr, &num))
> + tprintf("%" PRIu8, num);
> + break;
> + }
> + case NLA_U16: {
> + uint16_t num;
> +
> + if (len < sizeof(num))
> + printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);
> + else if (!umove_or_printaddr(tcp, addr, &num))
> + tprintf("%" PRIu16, num);
> + break;
> + }
> + case NLA_U32: {
> + uint32_t num;
> +
> + if (len < sizeof(num))
> + printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);
> + else if (!umove_or_printaddr(tcp, addr, &num))
> + tprintf("%" PRIu32, num);
> + break;
> + }
> + case NLA_MSECS:
> + case NLA_U64: {
> + uint64_t num;
> +
> + if (len < sizeof(num))
> + printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);
> + else if (!umove_or_printaddr(tcp, addr, &num))
> + tprintf("%" PRIu64, num);
> + break;
> + }
> + case NLA_STRING: {
> + uint16_t str_len = len;
> +
> + if (policy[nla_type].len && policy[nla_type].len < len)
> + str_len = policy[nla_type].len;
> + printstrn(tcp, addr, str_len);
> + break;
> + }
> + case NLA_NUL_STRING:
> + printstr(tcp, addr);
> + break;
> + case NLA_S8: {
> + int8_t num;
> +
> + if (len < sizeof(num))
> + printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);
> + else if (!umove_or_printaddr(tcp, addr, &num))
> + tprintf("%" PRId8, num);
> + break;
> + }
> + case NLA_S16: {
> + int16_t num;
> +
> + if (len < sizeof(num))
> + printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);
> + else if (!umove_or_printaddr(tcp, addr, &num))
> + tprintf("%" PRId16, num);
> + break;
> + }
> + case NLA_S32: {
> + int32_t num;
> +
> + if (len < sizeof(num))
> + printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);
> + else if (!umove_or_printaddr(tcp, addr, &num))
> + tprintf("%" PRId32, num);
> + break;
> + }
> + case NLA_S64: {
> + int64_t num;
> +
> + if (len < sizeof(num))
> + printstr_ex(tcp, addr, len, QUOTE_FORCE_HEX);
> + else if (!umove_or_printaddr(tcp, addr, &num))
> + tprintf("%" PRId64, num);
> + break;
> + }
All these numeric cases look very similar, could you think of something
that would save us from these cut-n-paste lines?
> + case NLA_NESTED:
> + case NLA_NESTED_COMPAT:
> + case NLA_BINARY:
> + if (fallback_parser
> + && fallback_parser(tcp, addr, len, nla_type, opaque_data))
> + break;
> + /* fall through when fallback_parser return false */
> + case NLA_FLAG:
> + case NLA_UNSPEC:
> + default:
> + printstrn(tcp, addr, len);
> + break;
> + }
> +}
> +
> +static void
> +decode_nlattr_with_data(struct tcb *tcp,
> + kernel_ulong_t addr,
> + kernel_ulong_t len,
> + const struct xlat *table,
> + const char *dflt,
> + unsigned int policy_size,
> + const struct nla_policy *policy,
> + bool (*fallback_parser)(struct tcb *,
> + kernel_ulong_t addr,
> + kernel_ulong_t len,
> + int nla_type,
> + void *opaque_data),
> + void *const opaque_data)
> {
> struct nlattr nlattr;
>
> @@ -100,8 +229,10 @@ decode_nlattr_with_data(struct tcb *tcp, kernel_ulong_t addr,
> print_nlattr(&nlattr, table, dflt);
> if (nlattr.nla_len > NLA_HDRLEN) {
> tprints(", ");
> - printstrn(tcp, addr + NLA_HDRLEN,
> - nlattr.nla_len - NLA_HDRLEN);
> + decode_nlattr_data(tcp, addr + NLA_HDRLEN,
> + nlattr.nla_len - NLA_HDRLEN,
> + nlattr.nla_type, policy_size, policy,
> + fallback_parser, opaque_data);
> }
> tprints("}");
>
> @@ -115,13 +246,27 @@ decode_nlattr_with_data(struct tcb *tcp, kernel_ulong_t addr,
> }
>
> void
> -decode_nlattr(struct tcb *tcp, kernel_ulong_t addr, kernel_ulong_t len,
> - int hdrlen, const struct xlat *table, const char *dflt)
> +decode_nlattr(struct tcb *tcp,
> + kernel_ulong_t addr,
> + kernel_ulong_t len,
> + int hdrlen,
> + const struct xlat *table,
> + const char *dflt,
> + unsigned int policy_size,
> + const struct nla_policy *policy,
> + bool (*fallback_parser)(struct tcb *,
> + kernel_ulong_t addr,
> + kernel_ulong_t len,
> + int nla_type,
> + void *opaque_data),
> + void *const opaque_data)
> {
> if (len > NLMSG_ALIGN(hdrlen)) {
> tprints(", ");
> decode_nlattr_with_data(tcp, addr + NLMSG_ALIGN(hdrlen),
> - len - NLMSG_ALIGN(hdrlen), table, dflt);
> + len - NLMSG_ALIGN(hdrlen),
> + table, dflt, policy_size, policy,
> + fallback_parser, opaque_data);
> }
> }
>
> diff --git a/netlink_sock_diag.c b/netlink_sock_diag.c
> index 36c949b..dc8ff43 100644
> --- a/netlink_sock_diag.c
> +++ b/netlink_sock_diag.c
> @@ -143,7 +143,7 @@ decode_unix_diag_msg(struct tcb *const tcp,
> tprints("}");
>
> decode_nlattr(tcp, addr, len, sizeof(msg), unix_diag_attrs,
> - "UNIX_DIAG_???");
> + "UNIX_DIAG_???", 0, NULL, NULL, NULL);
> }
>
> static void
> @@ -226,7 +226,7 @@ decode_netlink_diag_msg(struct tcb *const tcp,
> tprints("}");
>
> decode_nlattr(tcp, addr, len, sizeof(msg), netlink_diag_attrs,
> - "NETLINK_DIAG_???");
> + "NETLINK_DIAG_???", 0, NULL, NULL, NULL);
> }
>
> static void
> @@ -291,7 +291,7 @@ decode_packet_diag_msg(struct tcb *const tcp,
> tprints("}");
>
> decode_nlattr(tcp, addr, len, sizeof(msg), packet_diag_attrs,
> - "PACKET_DIAG_???");
> + "PACKET_DIAG_???", 0, NULL, NULL, NULL);
> }
>
> static void
> @@ -349,7 +349,7 @@ decode_inet_diag_req_compat(struct tcb *const tcp,
> tprints("}");
>
> decode_nlattr(tcp, addr, len, sizeof(req), inet_diag_req_attrs,
> - "INET_DIAG_REQ_???");
> + "INET_DIAG_REQ_???", 0, NULL, NULL, NULL);
> }
>
> static void
> @@ -387,7 +387,7 @@ decode_inet_diag_req_v2(struct tcb *const tcp,
> tprints("}");
>
> decode_nlattr(tcp, addr, len, sizeof(req), inet_diag_req_attrs,
> - "INET_DIAG_REQ_???");
> + "INET_DIAG_REQ_???", 0, NULL, NULL, NULL);
> }
>
> static void
> @@ -447,7 +447,7 @@ decode_inet_diag_msg(struct tcb *const tcp,
> tprints("}");
>
> decode_nlattr(tcp, addr, len, sizeof(msg), inet_diag_attrs,
> - "INET_DIAG_???");
> + "INET_DIAG_???", 0, NULL, NULL, NULL);
> }
>
> #ifdef AF_SMC
> @@ -521,7 +521,7 @@ decode_smc_diag_msg(struct tcb *const tcp,
> tprints("}");
>
> decode_nlattr(tcp, addr, len, sizeof(msg), smc_diag_attrs,
> - "SMC_DIAG_???");
> + "SMC_DIAG_???", 0, NULL, NULL, NULL);
> }
> #endif
>
--
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20170617/09976135/attachment.bin>
More information about the Strace-devel
mailing list