| Message ID | 20170719143257.3772-1-phil@nwl.cc |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
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. Also applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 24, 2017 at 01:17:30PM +0200, Pablo Neira Ayuso wrote: > 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. All ACK, one remark: [...] > > + 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. NFTA_GEN_PROC_NAME attribute is filled with output from get_task_comm(), which returns a string of max 16 bytes length. It is safe to assume that it's NULL terminated since set_task_comm() uses strlcpy(). That static buffer above is needless though, so I'll change it to just point to the netlink attribute itself if it is present. Thanks, Phil -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h index 683f6f88fcace..40096de04e963 100644 --- a/include/linux/netfilter/nf_tables.h +++ b/include/linux/netfilter/nf_tables.h @@ -1221,6 +1221,8 @@ enum nft_objref_attributes { enum nft_gen_attributes { NFTA_GEN_UNSPEC, NFTA_GEN_ID, + NFTA_GEN_PROC_ID, + NFTA_GEN_PROC_NAME, __NFTA_GEN_MAX }; #define NFTA_GEN_MAX (__NFTA_GEN_MAX - 1) 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; + 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)); + 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); + if (pid >= 0) { + printf(" by process %d", pid); + if (!monh->ctx->octx->numeric) + printf(" (%s)", name); + } + printf("\n"); + + return MNL_CB_OK; +} + static int netlink_events_cb(const struct nlmsghdr *nlh, void *data) { int ret = MNL_CB_OK; @@ -2955,6 +2992,9 @@ static int netlink_events_cb(const struct nlmsghdr *nlh, void *data) case NFT_MSG_DELOBJ: ret = netlink_events_obj_cb(nlh, type, monh); break; + case NFT_MSG_NEWGEN: + ret = netlink_events_newgen_cb(nlh, type, monh); + break; } fflush(stdout);
Now that they contain process information, they're actually interesting. For backwards compatibility, print process information only if it was present in the message. Signed-off-by: Phil Sutter <phil@nwl.cc> --- include/linux/netfilter/nf_tables.h | 2 ++ src/netlink.c | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+)