Please, add description to your patch, something like:
Move nfnetlink osf subsystem to standalone module so we can reuse it
from the new nft_ost extension.
More comments below.
On Tue, Jul 17, 2018 at 07:25:17PM +0200, Fernando Fernandez Mancera wrote:
> Signed-off-by: Fernando Fernandez Mancera <ffmancera@xxxxxxxxxx>
> ---
> include/linux/netfilter/nf_osf.h | 2 +
> include/uapi/linux/netfilter/nf_osf.h | 5 +
> include/uapi/linux/netfilter/xt_osf.h | 8 --
> net/netfilter/Kconfig | 10 +-
> net/netfilter/Makefile | 1 +
> net/netfilter/nf_osf.c | 7 ++
> net/netfilter/nfnetlink_osf.c | 134 ++++++++++++++++++++++++++
> net/netfilter/xt_osf.c | 133 ++-----------------------
> 8 files changed, 164 insertions(+), 136 deletions(-)
> create mode 100644 net/netfilter/nfnetlink_osf.c
>
> diff --git a/include/linux/netfilter/nf_osf.h b/include/linux/netfilter/nf_osf.h
> index 7d0947d6ef16..3411c87152d7 100644
> --- a/include/linux/netfilter/nf_osf.h
> +++ b/include/linux/netfilter/nf_osf.h
> @@ -27,6 +27,8 @@ struct nf_osf_finger {
> struct nf_osf_user_finger finger;
> };
>
> +extern struct list_head nf_osf_fingers[2];
> +
> bool nf_osf_match(const struct sk_buff *skb, u_int8_t family,
> int hooknum, struct net_device *in, struct net_device *out,
> const struct nf_osf_info *info, struct net *net,
> diff --git a/include/uapi/linux/netfilter/nf_osf.h b/include/uapi/linux/netfilter/nf_osf.h
> index a89583099b2a..18516829fce1 100644
> --- a/include/uapi/linux/netfilter/nf_osf.h
> +++ b/include/uapi/linux/netfilter/nf_osf.h
> @@ -96,4 +96,9 @@ enum nf_osf_attr_type {
> OSF_ATTR_MAX,
> };
>
> +enum nf_osf_msg_types {
> + OSF_MSG_ADD,
> + OSF_MSG_REMOVE,
> + OSF_MSG_MAX,
> +};
> #endif /* _NF_OSF_H */
> diff --git a/include/uapi/linux/netfilter/xt_osf.h b/include/uapi/linux/netfilter/xt_osf.h
> index b189007f4f28..6fe4d75b1f11 100644
> --- a/include/uapi/linux/netfilter/xt_osf.h
> +++ b/include/uapi/linux/netfilter/xt_osf.h
> @@ -47,13 +47,5 @@
> #define xt_osf_nlmsg nf_osf_nlmsg
>
> #define xt_osf_attr_type nf_osf_attr_type
> -/*
> - * Add/remove fingerprint from the kernel.
> - */
> -enum xt_osf_msg_types {
> - OSF_MSG_ADD,
> - OSF_MSG_REMOVE,
> - OSF_MSG_MAX,
> -};
I think you're missing something like:
#define xt_osf_attr_type nf_osf_attr_type
here.
> #endif /* _XT_OSF_H */
> diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
> index 6c65d756e603..06abc3bc05d3 100644
> --- a/net/netfilter/Kconfig
> +++ b/net/netfilter/Kconfig
> @@ -46,6 +46,14 @@ config NETFILTER_NETLINK_LOG
> and is also scheduled to replace the old syslog-based ipt_LOG
> and ip6t_LOG modules.
>
> +config NETFILTER_NETLINK_OSF
> + tristate "Netfilter OSF over NFNETLINK interface"
> + depends on NETFILTER_ADVANCED
> + select NETFILTER_NETLINK
> + help
> + If this option is enabled, the kernel will include support
> + for passive OS fingerprint via NFNETLINK.
> +
> config NF_CONNTRACK
> tristate "Netfilter connection tracking support"
> default m if NETFILTER_ADVANCED=n
> @@ -1379,8 +1387,8 @@ config NETFILTER_XT_MATCH_NFACCT
>
> config NETFILTER_XT_MATCH_OSF
> tristate '"osf" Passive OS fingerprint match'
> - depends on NETFILTER_ADVANCED && NETFILTER_NETLINK
> select NF_OSF
> + select NETFILTER_NETLINK_OSF
> help
> This option selects the Passive OS Fingerprinting match module
> that allows to passively match the remote operating system by
> diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
> index 0b3851e825fa..ab144e65edf1 100644
> --- a/net/netfilter/Makefile
> +++ b/net/netfilter/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_NETFILTER_NETLINK) += nfnetlink.o
> obj-$(CONFIG_NETFILTER_NETLINK_ACCT) += nfnetlink_acct.o
> obj-$(CONFIG_NETFILTER_NETLINK_QUEUE) += nfnetlink_queue.o
> obj-$(CONFIG_NETFILTER_NETLINK_LOG) += nfnetlink_log.o
> +obj-$(CONFIG_NETFILTER_NETLINK_OSF) += nfnetlink_osf.o
>
> # connection tracking
> obj-$(CONFIG_NF_CONNTRACK) += nf_conntrack.o
> diff --git a/net/netfilter/nf_osf.c b/net/netfilter/nf_osf.c
> index f4c75e982902..b5c3af060d62 100644
> --- a/net/netfilter/nf_osf.c
> +++ b/net/netfilter/nf_osf.c
> @@ -20,6 +20,13 @@
> #include <net/netfilter/nf_log.h>
> #include <linux/netfilter/nf_osf.h>
>
> +/*
> + * Indexed by dont-fragment bit.
> + * It is the only constant value in the fingerprint.
> + */
> +struct list_head nf_osf_fingers[2];
> +EXPORT_SYMBOL_GPL(nf_osf_fingers);
> +
> static inline int nf_osf_ttl(const struct sk_buff *skb,
> int ttl_check, unsigned char f_ttl)
> {
> diff --git a/net/netfilter/nfnetlink_osf.c b/net/netfilter/nfnetlink_osf.c
> new file mode 100644
> index 000000000000..ba71f04a4004
> --- /dev/null
> +++ b/net/netfilter/nfnetlink_osf.c
> @@ -0,0 +1,134 @@
> +#include <linux/netfilter/nfnetlink.h>
> +#include <linux/netfilter/nf_osf.h>
> +
> +static const struct nla_policy nfnl_osf_policy[OSF_ATTR_MAX + 1] = {
> + [OSF_ATTR_FINGER] = { .len = sizeof(struct nf_osf_user_finger) },
> +};
> +
> +static int nfnl_osf_add_callback(struct net *net, struct sock *ctnl,
> + struct sk_buff *skb, const struct nlmsghdr *nlh,
> + const struct nlattr * const osf_attrs[],
> + struct netlink_ext_ack *extack)
> +{
> + struct nf_osf_user_finger *f;
> + struct nf_osf_finger *kf = NULL, *sf;
> + int err = 0;
> +
> + if (!capable(CAP_NET_ADMIN))
> + return -EPERM;
> +
> + if (!osf_attrs[OSF_ATTR_FINGER])
> + return -EINVAL;
> +
> + if (!(nlh->nlmsg_flags & NLM_F_CREATE))
> + return -EINVAL;
> +
> + f = nla_data(osf_attrs[OSF_ATTR_FINGER]);
> +
> + kf = kmalloc(sizeof(struct nf_osf_finger), GFP_KERNEL);
> + if (!kf)
> + return -ENOMEM;
> +
> + memcpy(&kf->finger, f, sizeof(struct nf_osf_user_finger));
> +
> + list_for_each_entry(sf, &nf_osf_fingers[!!f->df], finger_entry) {
> + if (memcmp(&sf->finger, f, sizeof(struct nf_osf_user_finger)))
> + continue;
> +
> + kfree(kf);
> + kf = NULL;
> +
> + if (nlh->nlmsg_flags & NLM_F_EXCL)
> + err = -EEXIST;
> + break;
> + }
> +
> + /*
> + * We are protected by nfnl mutex.
> + */
> + if (kf)
> + list_add_tail_rcu(&kf->finger_entry, &nf_osf_fingers[!!f->df]);
> +
> + return err;
> +}
> +
> +static int nfnl_osf_remove_callback(struct net *net, struct sock *ctnl,
> + struct sk_buff *skb,
> + const struct nlmsghdr *nlh,
> + const struct nlattr * const osf_attrs[],
> + struct netlink_ext_ack *extack)
> +{
> + struct nf_osf_user_finger *f;
> + struct nf_osf_finger *sf;
> + int err = -ENOENT;
> +
> + if (!capable(CAP_NET_ADMIN))
> + return -EPERM;
> +
> + if (!osf_attrs[OSF_ATTR_FINGER])
> + return -EINVAL;
> +
> + f = nla_data(osf_attrs[OSF_ATTR_FINGER]);
> +
> + list_for_each_entry(sf, &nf_osf_fingers[!!f->df], finger_entry) {
> + if (memcmp(&sf->finger, f, sizeof(struct nf_osf_user_finger)))
> + continue;
> +
> + /*
> + * We are protected by nfnl mutex.
> + */
> + list_del_rcu(&sf->finger_entry);
> + kfree_rcu(sf, rcu_head);
> +
> + err = 0;
> + break;
> + }
> +
> + return err;
> +}
> +
> +static const struct nfnl_callback nfnl_nf_osf_callbacks[OSF_MSG_MAX] = {
> + [OSF_MSG_ADD] = {
> + .call = nfnl_osf_add_callback,
> + .attr_count = OSF_ATTR_MAX,
> + .policy = nfnl_osf_policy,
> + },
> + [OSF_MSG_REMOVE] = {
> + .call = nfnl_osf_remove_callback,
> + .attr_count = OSF_ATTR_MAX,
> + .policy = nfnl_osf_policy,
> + },
> +};
> +
> +static const struct nfnetlink_subsystem nfnl_osf_subsys = {
> + .name = "osf",
> + .subsys_id = NFNL_SUBSYS_OSF,
> + .cb_count = OSF_MSG_MAX,
> + .cb = nfnl_nf_osf_callbacks,
> +};
> +
> +static int __init nfnl_osf_init(void)
> +{
> + int err = -EINVAL;
> +
> + err = nfnetlink_subsys_register(&nfnl_osf_subsys);
> + if (err < 0) {
> + pr_err("Failed to register OSF nsfnetlink helper (%d)\n", err);
> + goto err_out_exit;
> + }
> +
> + return 0;
> +
> +err_out_exit:
> + return err;
> +}
> +
> +static void __exit nfnl_osf_fini(void)
> +{
> + nfnetlink_subsys_unregister(&nfnl_osf_subsys);
> +}
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Fernando Fernandez <ffmancera@xxxxxxxxxx>");
> +module_init(nfnl_osf_init);
> +module_exit(nfnl_osf_fini);
> diff --git a/net/netfilter/xt_osf.c b/net/netfilter/xt_osf.c
> index 9cfef73b4107..a7dcfb9835c4 100644
> --- a/net/netfilter/xt_osf.c
> +++ b/net/netfilter/xt_osf.c
> @@ -37,118 +37,6 @@
> #include <net/netfilter/nf_log.h>
> #include <linux/netfilter/xt_osf.h>
>
> -/*
> - * Indexed by dont-fragment bit.
> - * It is the only constant value in the fingerprint.
> - */
> -static struct list_head xt_osf_fingers[2];
> -
> -static const struct nla_policy xt_osf_policy[OSF_ATTR_MAX + 1] = {
> - [OSF_ATTR_FINGER] = { .len = sizeof(struct xt_osf_user_finger) },
> -};
> -
> -static int xt_osf_add_callback(struct net *net, struct sock *ctnl,
> - struct sk_buff *skb, const struct nlmsghdr *nlh,
> - const struct nlattr * const osf_attrs[],
> - struct netlink_ext_ack *extack)
> -{
> - struct xt_osf_user_finger *f;
> - struct xt_osf_finger *kf = NULL, *sf;
> - int err = 0;
> -
> - if (!capable(CAP_NET_ADMIN))
> - return -EPERM;
> -
> - if (!osf_attrs[OSF_ATTR_FINGER])
> - return -EINVAL;
> -
> - if (!(nlh->nlmsg_flags & NLM_F_CREATE))
> - return -EINVAL;
> -
> - f = nla_data(osf_attrs[OSF_ATTR_FINGER]);
> -
> - kf = kmalloc(sizeof(struct xt_osf_finger), GFP_KERNEL);
> - if (!kf)
> - return -ENOMEM;
> -
> - memcpy(&kf->finger, f, sizeof(struct xt_osf_user_finger));
> -
> - list_for_each_entry(sf, &xt_osf_fingers[!!f->df], finger_entry) {
> - if (memcmp(&sf->finger, f, sizeof(struct xt_osf_user_finger)))
> - continue;
> -
> - kfree(kf);
> - kf = NULL;
> -
> - if (nlh->nlmsg_flags & NLM_F_EXCL)
> - err = -EEXIST;
> - break;
> - }
> -
> - /*
> - * We are protected by nfnl mutex.
> - */
> - if (kf)
> - list_add_tail_rcu(&kf->finger_entry, &xt_osf_fingers[!!f->df]);
> -
> - return err;
> -}
> -
> -static int xt_osf_remove_callback(struct net *net, struct sock *ctnl,
> - struct sk_buff *skb,
> - const struct nlmsghdr *nlh,
> - const struct nlattr * const osf_attrs[],
> - struct netlink_ext_ack *extack)
> -{
> - struct xt_osf_user_finger *f;
> - struct xt_osf_finger *sf;
> - int err = -ENOENT;
> -
> - if (!capable(CAP_NET_ADMIN))
> - return -EPERM;
> -
> - if (!osf_attrs[OSF_ATTR_FINGER])
> - return -EINVAL;
> -
> - f = nla_data(osf_attrs[OSF_ATTR_FINGER]);
> -
> - list_for_each_entry(sf, &xt_osf_fingers[!!f->df], finger_entry) {
> - if (memcmp(&sf->finger, f, sizeof(struct xt_osf_user_finger)))
> - continue;
> -
> - /*
> - * We are protected by nfnl mutex.
> - */
> - list_del_rcu(&sf->finger_entry);
> - kfree_rcu(sf, rcu_head);
> -
> - err = 0;
> - break;
> - }
> -
> - return err;
> -}
> -
> -static const struct nfnl_callback xt_osf_nfnetlink_callbacks[OSF_MSG_MAX] = {
> - [OSF_MSG_ADD] = {
> - .call = xt_osf_add_callback,
> - .attr_count = OSF_ATTR_MAX,
> - .policy = xt_osf_policy,
> - },
> - [OSF_MSG_REMOVE] = {
> - .call = xt_osf_remove_callback,
> - .attr_count = OSF_ATTR_MAX,
> - .policy = xt_osf_policy,
> - },
> -};
> -
> -static const struct nfnetlink_subsystem xt_osf_nfnetlink = {
> - .name = "osf",
> - .subsys_id = NFNL_SUBSYS_OSF,
> - .cb_count = OSF_MSG_MAX,
> - .cb = xt_osf_nfnetlink_callbacks,
> -};
> -
> static bool
> xt_osf_match_packet(const struct sk_buff *skb, struct xt_action_param *p)
> {
> @@ -159,7 +47,7 @@ xt_osf_match_packet(const struct sk_buff *skb, struct xt_action_param *p)
> return false;
>
> return nf_osf_match(skb, xt_family(p), xt_hooknum(p), xt_in(p),
> - xt_out(p), info, net, xt_osf_fingers);
> + xt_out(p), info, net, nf_osf_fingers);
> }
>
> static struct xt_match xt_osf_match = {
> @@ -180,26 +68,18 @@ static int __init xt_osf_init(void)
> int err = -EINVAL;
> int i;
>
> - for (i=0; i<ARRAY_SIZE(xt_osf_fingers); ++i)
> - INIT_LIST_HEAD(&xt_osf_fingers[i]);
> -
> - err = nfnetlink_subsys_register(&xt_osf_nfnetlink);
> - if (err < 0) {
> - pr_err("Failed to register OSF nsfnetlink helper (%d)\n", err);
> - goto err_out_exit;
> - }
> + for (i = 0; i < ARRAY_SIZE(nf_osf_fingers); ++i)
> + INIT_LIST_HEAD(&nf_osf_fingers[i]);
Please, do this initialization from nfnetlink_osf.
> err = xt_register_match(&xt_osf_match);
> if (err) {
> pr_err("Failed to register OS fingerprint "
> "matching module (%d)\n", err);
> - goto err_out_remove;
> + goto err_out_exit;
> }
>
> return 0;
>
> -err_out_remove:
> - nfnetlink_subsys_unregister(&xt_osf_nfnetlink);
> err_out_exit:
> return err;
> }
> @@ -209,13 +89,12 @@ static void __exit xt_osf_fini(void)
> struct xt_osf_finger *f;
> int i;
>
> - nfnetlink_subsys_unregister(&xt_osf_nfnetlink);
> xt_unregister_match(&xt_osf_match);
>
> rcu_read_lock();
> - for (i=0; i<ARRAY_SIZE(xt_osf_fingers); ++i) {
> + for (i = 0; i < ARRAY_SIZE(nf_osf_fingers); ++i) {
>
> - list_for_each_entry_rcu(f, &xt_osf_fingers[i], finger_entry) {
> + list_for_each_entry_rcu(f, &nf_osf_fingers[i], finger_entry) {
> list_del_rcu(&f->finger_entry);
> kfree_rcu(f, rcu_head);
> }
This for loop should go to nfnetlink too.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
