On Wed, Jul 19, 2017 at 04:32:57PM +0200, Phil Sutter wrote:
> Now that they contain process information, they're actually interesting.
> For backwards compatibility, print process information only if it was
> present in the message.
Wait, a couple of comments.
[...]
> diff --git a/src/netlink.c b/src/netlink.c
> index e3c90dac8c7a6..44a9806097d00 100644
> --- a/src/netlink.c
> +++ b/src/netlink.c
> @@ -2915,6 +2915,43 @@ static void netlink_events_debug(uint16_t type)
> #endif /* DEBUG */
> }
>
> +static int netlink_events_newgen_cb(const struct nlmsghdr *nlh, int type,
> + struct netlink_mon_handler *monh)
> +{
> + const struct nlattr *attr;
> + char name[256] = "";
> + int genid, pid = -1;
> +
> + mnl_attr_for_each(attr, nlh, sizeof(struct nfgenmsg)) {
> + switch (mnl_attr_get_type(attr)) {
> + case NFTA_GEN_ID:
> + if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0)
> + break;
I think it's better to hit netlink_abi_error() error. If validation
fails, it means someone in the kernel has accidentally changed the
size/type of this attribute. This it's basically breaking the ABI.
Better bail out here so people come to us so we quickly fix this.
> + genid = ntohl(mnl_attr_get_u32(attr));
> + break;
> + case NFTA_GEN_PROC_NAME:
> + if (mnl_attr_validate(attr, MNL_TYPE_NUL_STRING) < 0)
> + break;
> + strncpy(name, mnl_attr_get_str(attr), sizeof(name));
What is maximum process name length? If we hit this bound, we have to
make sure this does:
name[X - 1] = '\0';
Where X is the name buffer size.
> + break;
> + case NFTA_GEN_PROC_ID:
> + if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0)
> + break;
> + pid = ntohl(mnl_attr_get_u32(attr));
> + break;
> + }
> + }
> + printf("new generation %d", genid);
Please, prepend '#' to this message.
> + if (pid >= 0) {
> + printf(" by process %d", pid);
> + if (!monh->ctx->octx->numeric)
> + printf(" (%s)", name);
> + }
> + printf("\n");
> +
> + return MNL_CB_OK;
> +}
Thanks!
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
