[PATCH nf-next 1/1] netfilter: ctlink: Return error directly when create expect without help
From: Gao Feng
The expect check func "__nf_ct_expect_check" asks the master_help is
necessary. So it is unnecessary to go ahead in ctnetlink_alloc_expect
when there is no help.
Actually the commit bc01befdcf3e ("netfilter: ctnetlink: add support
for user-space expectation helpers") permits ctlink create one expect
even though there is no master help. But the latter commit 3d058d7bc2c5
("netfilter: rework user-space expectation helper support") disables
it again.
Signed-off-by: Gao Feng
---
net/netfilter/nf_conntrack_netlink.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/net/netfilter/nf_conntrack_netlink.c
b/net/netfilter/nf_conntrack_netlink.c
index d49cc1e..c9af545 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -3049,18 +3049,8 @@ static int ctnetlink_del_expect(struct net *net, struct
sock *ctnl,
help = nfct_help(ct);
if (!help) {
- if (!cda[CTA_EXPECT_TIMEOUT]) {
- err = -EINVAL;
- goto err_out;
- }
- exp->timeout.expires =
- jiffies + ntohl(nla_get_be32(cda[CTA_EXPECT_TIMEOUT])) * HZ;
-
- exp->flags = NF_CT_EXPECT_USERSPACE;
- if (cda[CTA_EXPECT_FLAGS]) {
- exp->flags |=
- ntohl(nla_get_be32(cda[CTA_EXPECT_FLAGS]));
- }
+ err = -EOPNOTSUPP;
+ goto err_out;
} else {
if (cda[CTA_EXPECT_FLAGS]) {
exp->flags = ntohl(nla_get_be32(cda[CTA_EXPECT_FLAGS]));
--
1.9.1
--
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
[PATCH v2] net: netfilter: Remove multiple assignment.
This patch removes multiple assignments to follow the kernel coding
style as also reported by checkpatch.pl.
Done using coccinelle.
@@
identifier i1,i2;
constant c;
@@
- i1=i2=c;
+ i1=c;
+ i2=i1;
Signed-off-by: Arushi Singhal
---
changes in v2
-Make the commit message more clear and appropriate.
net/netfilter/nf_conntrack_proto_sctp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nf_conntrack_proto_sctp.c
b/net/netfilter/nf_conntrack_proto_sctp.c
index 33279aab583d..723386bcc2cb 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -346,7 +346,8 @@ static int sctp_packet(struct nf_conn *ct,
goto out;
}
- old_state = new_state = SCTP_CONNTRACK_NONE;
+ old_state = SCTP_CONNTRACK_NONE;
+ new_state = old_state;
spin_lock_bh(&ct->lock);
for_each_sctp_chunk (skb, sch, _sch, offset, dataoff, count) {
/* Special cases of Verification tag check (Sec 8.5.1) */
--
2.11.0
--
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
Re: [Outreachy kernel] Re: [PATCH] iptables: Constify option struct
On Tue, 28 Mar 2017, Gargi Sharma wrote:
> On Tue, Mar 28, 2017 at 12:45 AM, Gargi Sharma wrote:
> > On Tue, Mar 28, 2017 at 12:42 AM, Pablo Neira Ayuso
> > wrote:
> >>
> >> On Tue, Mar 28, 2017 at 12:40:15AM +0530, Gargi Sharma wrote:
> >> > The struct of the type option is only used to initialise a field
> >> > inside the xtables_match struct and is not modified anywhere.
> >> > Done using Coccinelle:
> >> >
> >> > @r1 disable optional_qualifier@
> >> > identifier s,i;
> >> > position p;
> >> > @@
> >> >
> >> > static struct option i@p[] ={...};
> >> >
> >> > @ok1@
> >> > identifier r1.i;
> >> > expression e;
> >> > position p;
> >> > @@
> >> > e = i@p
> >> >
> >> > @bad@
> >> > position p != {r1.p,ok1.p};
> >> > identifier r1.i;
> >> > @@
> >> > e@i@p
> >> >
> >> > @depends on !bad disable optional_qualifier@
> >> > identifier r1.i;
> >> > @@
> >> > static
> >> > +const
> >> > struct option i[] = { ... };
> >>
> >> Applied, thanks.
> >>
> >> You can probably have a look at ebtables.git, probably there are more
> >> things to constify there.
>
> Hi Pablo,
>
> There are around 18 files being affected by this semantic patch in
> ebtables. Should I still send them as a single patch(since it might be
> difficult to review a large single patch)?
If they are just independent structures, you can cut up the patches if it
seems best. If the changes are interdependent, then one patch would be
better.
julia
>
> Thanks!
> Gargi
> >>
> > Okay, I will. :)
> >
> > Thanks!
> > Gargi
> >> --
> >> You received this message because you are subscribed to the Google Groups
> >> "outreachy-kernel" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an
> >> email to outreachy-kernel+unsubscr...@googlegroups.com.
> >> To post to this group, send email to outreachy-ker...@googlegroups.com.
> >> To view this discussion on the web visit
> >> https://groups.google.com/d/msgid/outreachy-kernel/20170327191239.GA18080%40salvia.
> >> For more options, visit https://groups.google.com/d/optout.
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/outreachy-kernel/CAOCi2DFc2aURc5gAuF5q6NLvBwiVSbF6cpoF%3DipfVwntz%3D86Hg%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
--
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
Re: [PATCH 3/5] ipset: hash:ipport: netmask support
On Tue, 21 Mar 2017, Josh Hunt wrote:
> Adds netmask support to hash:ipport sets.
>
> Signed-off-by: Josh Hunt
> ---
> lib/ipset_hash_ipport.c | 194
>
> 1 file changed, 194 insertions(+)
>
> diff --git a/lib/ipset_hash_ipport.c b/lib/ipset_hash_ipport.c
> index 2166922..a2cf79e 100644
> --- a/lib/ipset_hash_ipport.c
> +++ b/lib/ipset_hash_ipport.c
> @@ -787,6 +787,199 @@ static struct ipset_type ipset_hash_ipport5 = {
> .description = "skbinfo support",
> };
>
> +/* Parse commandline arguments */
> +static const struct ipset_arg hash_ipport_create_args6[] = {
> + { .name = { "family", NULL },
> + .has_arg = IPSET_MANDATORY_ARG, .opt = IPSET_OPT_FAMILY,
> + .parse = ipset_parse_family, .print = ipset_print_family,
> + },
> + /* Alias: family inet */
> + { .name = { "-4", NULL },
> + .has_arg = IPSET_NO_ARG, .opt = IPSET_OPT_FAMILY,
> + .parse = ipset_parse_family,
> + },
> + /* Alias: family inet6 */
> + { .name = { "-6", NULL },
> + .has_arg = IPSET_NO_ARG, .opt = IPSET_OPT_FAMILY,
> + .parse = ipset_parse_family,
> + },
> + { .name = { "hashsize", NULL },
> + .has_arg = IPSET_MANDATORY_ARG, .opt = IPSET_OPT_HASHSIZE,
> + .parse = ipset_parse_uint32, .print = ipset_print_number,
> + },
> + { .name = { "maxelem", NULL },
> + .has_arg = IPSET_MANDATORY_ARG, .opt = IPSET_OPT_MAXELEM,
> + .parse = ipset_parse_uint32, .print = ipset_print_number,
> + },
> + { .name = { "timeout", NULL },
> + .has_arg = IPSET_MANDATORY_ARG, .opt = IPSET_OPT_TIMEOUT,
> + .parse = ipset_parse_timeout, .print = ipset_print_number,
> + },
> + { .name = { "counters", NULL },
> + .has_arg = IPSET_NO_ARG, .opt = IPSET_OPT_COUNTERS,
> + .parse = ipset_parse_flag,.print = ipset_print_flag,
> + },
> + { .name = { "comment", NULL },
> + .has_arg = IPSET_NO_ARG, .opt = IPSET_OPT_CREATE_COMMENT,
> + .parse = ipset_parse_flag,.print = ipset_print_flag,
> + },
> + { .name = { "forceadd", NULL },
> + .has_arg = IPSET_NO_ARG, .opt = IPSET_OPT_FORCEADD,
> + .parse = ipset_parse_flag,.print = ipset_print_flag,
> + },
> + { .name = { "skbinfo", NULL },
> + .has_arg = IPSET_NO_ARG, .opt = IPSET_OPT_SKBINFO,
> + .parse = ipset_parse_flag,.print = ipset_print_flag,
> + },
> + { .name = { "netmask", NULL },
> + .has_arg = IPSET_MANDATORY_ARG, .opt = IPSET_OPT_NETMASK_MASK,
> + .parse = ipset_parse_netmask, .print = ipset_print_netmask,
With the modified parser you can use IPSET_OPT_NETMASK here - and the same
comment for the hash:ip,port type.
> + },
> + /* Backward compatibility */
> + { .name = { "probes", NULL },
> + .has_arg = IPSET_MANDATORY_ARG, .opt = IPSET_OPT_PROBES,
> + .parse = ipset_parse_ignored, .print = ipset_print_number,
> + },
> + { .name = { "resize", NULL },
> + .has_arg = IPSET_MANDATORY_ARG, .opt = IPSET_OPT_RESIZE,
> + .parse = ipset_parse_ignored, .print = ipset_print_number,
> + },
> + { .name = { "from", NULL },
> + .has_arg = IPSET_MANDATORY_ARG, .opt = IPSET_OPT_IP,
> + .parse = ipset_parse_ignored,
> + },
> + { .name = { "to", NULL },
> + .has_arg = IPSET_MANDATORY_ARG, .opt = IPSET_OPT_IP_TO,
> + .parse = ipset_parse_ignored,
> + },
> + { .name = { "network", NULL },
> + .has_arg = IPSET_MANDATORY_ARG, .opt = IPSET_OPT_IP,
> + .parse = ipset_parse_ignored,
> + },
> + { },
> +};
> +
> +static const struct ipset_arg hash_ipport_add_args6[] = {
> + { .name = { "timeout", NULL },
> + .has_arg = IPSET_MANDATORY_ARG, .opt = IPSET_OPT_TIMEOUT,
> + .parse = ipset_parse_timeout, .print = ipset_print_number,
> + },
> + { .name = { "packets", NULL },
> + .has_arg = IPSET_MANDATORY_ARG, .opt = IPSET_OPT_PACKETS,
> + .parse = ipset_parse_uint64, .print = ipset_print_number,
> + },
> + { .name = { "bytes", NULL },
> + .has_arg = IPSET_MANDATORY_ARG, .opt = IPSET_OPT_BYTES,
> + .parse = ipset_parse_uint64, .print = ipset_print_number,
> + },
> + { .name = { "comment", NULL },
> + .has_arg = IPSET_MANDATORY_ARG, .opt = IPSET_OPT_ADT_COMMENT,
> + .parse = ipset_parse_comment, .print = ipset_print_comment,
> + },
> + { .name = { "skbmark", NULL },
> + .has_arg = IPSET_MANDATORY_ARG, .opt = IPSET_OPT_SKBMARK,
> + .parse = ipset_parse_skbmark, .print = ipset_print_skbmark,
> + },
> + { .name = { "skbprio", NULL },
> + .has_arg
Re: [PATCH 1/5] ipset: netmask: expand to support cidr and full mask
On Tue, 21 Mar 2017, Josh Hunt wrote:
> Convert netmask to store cidr and netmask.
>
> Signed-off-by: Josh Hunt
> ---
> include/libipset/data.h | 5 ++-
> include/libipset/linux_ip_set.h | 5 +++
> include/libipset/print.h| 3 ++
> lib/data.c | 25 +--
> lib/debug.c | 1 +
> lib/parse.c | 68
> -
> lib/print.c | 39 +--
> lib/session.c | 8 +
> 8 files changed, 141 insertions(+), 13 deletions(-)
>
> diff --git a/include/libipset/data.h b/include/libipset/data.h
> index ca21890..0314cfb 100644
> --- a/include/libipset/data.h
> +++ b/include/libipset/data.h
> @@ -37,6 +37,7 @@ enum ipset_opt {
> IPSET_OPT_RESIZE,
> IPSET_OPT_SIZE,
> IPSET_OPT_FORCEADD,
> + IPSET_OPT_NETMASK_MASK,
> /* Create-specific options, filled out by the kernel */
> IPSET_OPT_ELEMENTS,
> IPSET_OPT_REFERENCES,
> @@ -66,6 +67,7 @@ enum ipset_opt {
> IPSET_OPT_SKBMARK,
> IPSET_OPT_SKBPRIO,
> IPSET_OPT_SKBQUEUE,
> + IPSET_OPT_NETMASK_FLAG,
> /* Internal options */
> IPSET_OPT_FLAGS = 48, /* IPSET_FLAG_EXIST| */
> IPSET_OPT_CADT_FLAGS, /* IPSET_FLAG_BEFORE| */
> @@ -101,7 +103,8 @@ enum ipset_opt {
> | IPSET_FLAG(IPSET_OPT_COUNTERS)\
> | IPSET_FLAG(IPSET_OPT_CREATE_COMMENT)\
> | IPSET_FLAG(IPSET_OPT_FORCEADD)\
> - | IPSET_FLAG(IPSET_OPT_SKBINFO))
> + | IPSET_FLAG(IPSET_OPT_SKBINFO)\
> + | IPSET_FLAG(IPSET_OPT_NETMASK_MASK))
>
> #define IPSET_ADT_FLAGS \
> (IPSET_FLAG(IPSET_OPT_IP) \
> diff --git a/include/libipset/linux_ip_set.h b/include/libipset/linux_ip_set.h
> index def91b9..4f93886 100644
> --- a/include/libipset/linux_ip_set.h
> +++ b/include/libipset/linux_ip_set.h
> @@ -84,6 +84,7 @@ enum {
> IPSET_ATTR_CADT_LINENO = IPSET_ATTR_LINENO, /* 9 */
> IPSET_ATTR_MARK,/* 10 */
> IPSET_ATTR_MARKMASK,/* 11 */
> + IPSET_ATTR_NETMASK_MASK,/* 12 */
> /* Reserve empty slots */
> IPSET_ATTR_CADT_MAX = 16,
> /* Create-only specific attributes */
> @@ -199,6 +200,8 @@ enum ipset_cadt_flags {
> IPSET_FLAG_WITH_FORCEADD = (1 << IPSET_FLAG_BIT_WITH_FORCEADD),
> IPSET_FLAG_BIT_WITH_SKBINFO = 6,
> IPSET_FLAG_WITH_SKBINFO = (1 << IPSET_FLAG_BIT_WITH_SKBINFO),
> + IPSET_FLAG_BIT_WITH_NETMASK = 7,
> + IPSET_FLAG_WITH_NETMASK = (1 << IPSET_FLAG_BIT_WITH_NETMASK),
> IPSET_FLAG_CADT_MAX = 15,
> };
>
> @@ -206,6 +209,8 @@ enum ipset_cadt_flags {
> enum ipset_create_flags {
> IPSET_CREATE_FLAG_BIT_FORCEADD = 0,
> IPSET_CREATE_FLAG_FORCEADD = (1 << IPSET_CREATE_FLAG_BIT_FORCEADD),
> + IPSET_CREATE_FLAG_BIT_NETMASK = 1,
> + IPSET_CREATE_FLAG_NETMASK = (1 << IPSET_CREATE_FLAG_BIT_NETMASK),
> IPSET_CREATE_FLAG_BIT_MAX = 7,
> };
>
> diff --git a/include/libipset/print.h b/include/libipset/print.h
> index 2103ce1..f9e77d3 100644
> --- a/include/libipset/print.h
> +++ b/include/libipset/print.h
> @@ -74,6 +74,9 @@ extern int ipset_print_flag(char *buf, unsigned int len,
> extern int ipset_print_elem(char *buf, unsigned int len,
> const struct ipset_data *data,
> enum ipset_opt opt, uint8_t env);
> +extern int ipset_print_netmask(char *buf, unsigned int len,
> + const struct ipset_data *data,
> + enum ipset_opt opt, uint8_t env);
>
> #define ipset_print_portnum ipset_print_number
>
> diff --git a/lib/data.c b/lib/data.c
> index 8372a2f..951a124 100644
> --- a/lib/data.c
> +++ b/lib/data.c
> @@ -24,6 +24,11 @@
> * We always store the data in host order, *except* IP addresses.
> */
>
> +struct ipset_netmask {
> + uint8_t cidr;
> + union nf_inet_addr mask;
> +};
> +
> struct ipset_data {
> /* Option bits: which fields are set */
> uint64_t bits;
> @@ -51,7 +56,7 @@ struct ipset_data {
> struct {
> uint8_t probes;
> uint8_t resize;
> - uint8_t netmask;
> + struct ipset_netmask netmask;
> uint32_t hashsize;
> uint32_t maxelem;
> uint32_t markmask;
> @@ -295,7 +300,13 @@ ipset_data_set(struct ipset_data *data, enum ipset_opt
> opt, const void *value)
> data->create.markmask = *(const uint32_t *) value;
> break;
> case IPSET_OPT_NETMASK:
> - data->create.netmask = *(const uint8_t *) value;
> + data->create.netmask.cidr = *(const uint8_t *) value;
> + break;
> + case IPSET_OPT_NETMASK_MASK:
> + if (!(data->family == NFPROTO_IPV4 ||
> + data->family == NFPROTO_IPV6))
> + return -1;
> +
Re: [PATCH 4/4] netfilter: ipset: hash:ipport: add netmask support
On Tue, 21 Mar 2017, Josh Hunt wrote:
> Initial netmask support for hash:ipport set types.
>
> Signed-off-by: Josh Hunt
> ---
> net/netfilter/ipset/ip_set_hash_ipport.c | 29 -
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/net/netfilter/ipset/ip_set_hash_ipport.c
> b/net/netfilter/ipset/ip_set_hash_ipport.c
> index f438740..39244c0 100644
> --- a/net/netfilter/ipset/ip_set_hash_ipport.c
> +++ b/net/netfilter/ipset/ip_set_hash_ipport.c
> @@ -29,7 +29,8 @@
> /* 2Counters support added */
> /* 3Comments support added */
> /* 4Forceadd support added */
> -#define IPSET_TYPE_REV_MAX 5 /* skbinfo support added */
> +/* 5skbinfo support added */
> +#define IPSET_TYPE_REV_MAX 6 /* netmask support added */
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Jozsef Kadlecsik ");
> @@ -38,6 +39,7 @@
>
> /* Type specific function prefix */
> #define HTYPEhash_ipport
> +#define IP_SET_HASH_WITH_NETMASK
>
> /* IPv4 variant */
>
> @@ -95,12 +97,16 @@ struct hash_ipport4_elem {
> ipset_adtfn adtfn = set->variant->adt[adt];
> struct hash_ipport4_elem e = { .ip = 0 };
> struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
> + const struct hash_ipport4 *h = set->data;
>
> if (!ip_set_get_ip4_port(skb, opt->flags & IPSET_DIM_TWO_SRC,
>&e.port, &e.proto))
> return -EINVAL;
>
> ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip);
> + e.ip &= h->netmask.mask.ip;
> + if (e.ip == 0)
> + return -EINVAL;
This checking is not needed, because only the fully zero valued hash
elements are not allowed and ip_set_get_ip4_port() excludes the zero
protocol value.
> return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags);
> }
>
> @@ -132,6 +138,10 @@ struct hash_ipport4_elem {
> if (ret)
> return ret;
>
> + e.ip &= h->netmask.mask.ip;
> + if (e.ip == 0)
> + return -EINVAL;
> +
The same here as at the hash_ipport4_kadt() function.
The handling of adding multiple elements from a range is missing here as
well. It's more complicated compared to the hash:ip type, because we must
allow to add multiple elements with a single IP address and a wildcard
mask value, but with a range of port numbers.
> e.port = nla_get_be16(tb[IPSET_ATTR_PORT]);
>
> if (tb[IPSET_ATTR_PROTO]) {
> @@ -237,6 +247,12 @@ struct hash_ipport6_elem {
> next->port = d->port;
> }
>
> +static inline void
> +hash_ipport6_netmask(union nf_inet_addr *ip, const union nf_inet_addr *mask)
> +{
> + nf_inet_addr_mask_inplace(ip, mask);
> +}
> +
> #undef MTYPE
> #undef HOST_MASK
>
> @@ -253,12 +269,17 @@ struct hash_ipport6_elem {
> ipset_adtfn adtfn = set->variant->adt[adt];
> struct hash_ipport6_elem e = { .ip = { .all = { 0 } } };
> struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
> + const struct hash_ipport6 *h = set->data;
>
> if (!ip_set_get_ip6_port(skb, opt->flags & IPSET_DIM_TWO_SRC,
>&e.port, &e.proto))
> return -EINVAL;
>
> ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6);
> + hash_ipport6_netmask(&e.ip, &h->netmask.mask);
> + if (ipv6_addr_any(&e.ip.in6))
> + return -EINVAL;
> +
The checking is unnecessary...
> return adtfn(set, &e, &ext, &opt->ext, opt->cmdflags);
> }
>
> @@ -298,6 +319,10 @@ struct hash_ipport6_elem {
> if (ret)
> return ret;
>
> + hash_ipport6_netmask(&e.ip, &h->netmask.mask);
> + if (ipv6_addr_any(&e.ip.in6))
> + return -EINVAL;
... and here as well.
> e.port = nla_get_be16(tb[IPSET_ATTR_PORT]);
>
> if (tb[IPSET_ATTR_PROTO]) {
> @@ -354,6 +379,8 @@ struct hash_ipport6_elem {
> [IPSET_ATTR_PROTO] = { .type = NLA_U8 },
> [IPSET_ATTR_TIMEOUT]= { .type = NLA_U32 },
> [IPSET_ATTR_CADT_FLAGS] = { .type = NLA_U32 },
> + [IPSET_ATTR_NETMASK]= { .type = NLA_U8 },
> + [IPSET_ATTR_NETMASK_MASK] = { .type = NLA_NESTED },
> },
> .adt_policy = {
> [IPSET_ATTR_IP] = { .type = NLA_NESTED },
> --
> 1.9.1
Best regards,
Jozsef
-
E-mail : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
H-1525 Budapest 114, POB. 49, Hungary
--
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
Re: [Outreachy kernel] Re: [PATCH] iptables: Constify option struct
On Tue, Mar 28, 2017 at 12:45 AM, Gargi Sharma wrote:
> On Tue, Mar 28, 2017 at 12:42 AM, Pablo Neira Ayuso
> wrote:
>>
>> On Tue, Mar 28, 2017 at 12:40:15AM +0530, Gargi Sharma wrote:
>> > The struct of the type option is only used to initialise a field
>> > inside the xtables_match struct and is not modified anywhere.
>> > Done using Coccinelle:
>> >
>> > @r1 disable optional_qualifier@
>> > identifier s,i;
>> > position p;
>> > @@
>> >
>> > static struct option i@p[] ={...};
>> >
>> > @ok1@
>> > identifier r1.i;
>> > expression e;
>> > position p;
>> > @@
>> > e = i@p
>> >
>> > @bad@
>> > position p != {r1.p,ok1.p};
>> > identifier r1.i;
>> > @@
>> > e@i@p
>> >
>> > @depends on !bad disable optional_qualifier@
>> > identifier r1.i;
>> > @@
>> > static
>> > +const
>> > struct option i[] = { ... };
>>
>> Applied, thanks.
>>
>> You can probably have a look at ebtables.git, probably there are more
>> things to constify there.
Hi Pablo,
There are around 18 files being affected by this semantic patch in
ebtables. Should I still send them as a single patch(since it might be
difficult to review a large single patch)?
Thanks!
Gargi
>>
> Okay, I will. :)
>
> Thanks!
> Gargi
>> --
>> You received this message because you are subscribed to the Google Groups
>> "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to outreachy-kernel+unsubscr...@googlegroups.com.
>> To post to this group, send email to outreachy-ker...@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/outreachy-kernel/20170327191239.GA18080%40salvia.
>> For more options, visit https://groups.google.com/d/optout.
--
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
Re: [PATCH 3/4] netfilter: ipset: hash:ip: add support for new netmask types
On Tue, 21 Mar 2017, Josh Hunt wrote:
> Enable new netmask suport for hash:ip set types.
>
> Example usage:
>
> Legacy behavior:
> ipset create foo hash:ip family inet6 netmask 64
>
> New netmask support (equivalent to legacy example):
> ipset create foo hash:ip family inet6 netmask :::::
>
> New wildcard mask support:
> ipset create foo hash:ip family inet6 netmask
> :::0:0:::
>
> The 3 mask types are supported for ipv4 sets as well.
>
> Signed-off-by: Josh Hunt
> ---
> net/netfilter/ipset/ip_set_hash_ip.c | 20 +++-
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/net/netfilter/ipset/ip_set_hash_ip.c
> b/net/netfilter/ipset/ip_set_hash_ip.c
> index 20bfbd3..74aa4a6 100644
> --- a/net/netfilter/ipset/ip_set_hash_ip.c
> +++ b/net/netfilter/ipset/ip_set_hash_ip.c
> @@ -27,7 +27,8 @@
> /* 1 Counters support */
> /* 2 Comments support */
> /* 3 Forceadd support */
> -#define IPSET_TYPE_REV_MAX 4 /* skbinfo support */
> +/* 4 skbinfo support */
> +#define IPSET_TYPE_REV_MAX 5 /* nf_inet_addr netmask support */
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Jozsef Kadlecsik ");
> @@ -89,7 +90,7 @@ struct hash_ip4_elem {
> __be32 ip;
>
> ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &ip);
> - ip &= ip_set_netmask(h->netmask);
> + ip &= h->netmask.mask.ip;
> if (ip == 0)
> return -EINVAL;
>
> @@ -122,7 +123,7 @@ struct hash_ip4_elem {
> if (ret)
> return ret;
>
> - ip &= ip_set_hostmask(h->netmask);
> + ip &= ntohl(h->netmask.mask.ip);
>
> if (adt == IPSET_TEST) {
> e.ip = htonl(ip);
> @@ -146,7 +147,7 @@ struct hash_ip4_elem {
> ip_set_mask_from_to(ip, ip_to, cidr);
> }
>
> - hosts = h->netmask == 32 ? 1 : 2 << (32 - h->netmask - 1);
> + hosts = (h->netmask.cidr == 32 || !h->netmask.cidr) ? 1 : 2 << (32 -
> h->netmask.cidr - 1);
This part is insufficient, because I dont't see how netmask.cidr could be
calculated when netmask.mask is an arbitrary wildcard mask. Also, the code
allows to add multiple elements in a range which is not handled properly.
What I'd like to see is as follows:
- If IPSET_ATTR_NETMASK_MASK attribute is passed then the set creation
routine should check whether the value can be converted to a cidr value
and if yes then set netmask.cidr.
- If netmask.cidr value is not valid (i.e. zero), then the add/del
loop for multiple elements should be skipped, i.e. instead of
if (adt == IPSET_TEST) {
it should be something like
if (adt == IPSET_TEST || !h->netmask.cidr) {
> if (retried)
> ip = ntohl(h->next.ip);
> @@ -182,9 +183,9 @@ struct hash_ip6_elem {
> }
>
> static inline void
> -hash_ip6_netmask(union nf_inet_addr *ip, u8 prefix)
> +hash_ip6_netmask(union nf_inet_addr *ip, const union nf_inet_addr *mask)
> {
> - ip6_netmask(ip, prefix);
> + nf_inet_addr_mask_inplace(ip, mask);
> }
>
> static bool
> @@ -223,7 +224,7 @@ struct hash_ip6_elem {
> struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
>
> ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, &e.ip.in6);
> - hash_ip6_netmask(&e.ip, h->netmask);
> + hash_ip6_netmask(&e.ip, &h->netmask.mask);
> if (ipv6_addr_any(&e.ip.in6))
> return -EINVAL;
>
> @@ -262,7 +263,7 @@ struct hash_ip6_elem {
> if (ret)
> return ret;
>
> - hash_ip6_netmask(&e.ip, h->netmask);
> + hash_ip6_netmask(&e.ip, &h->netmask.mask);
> if (ipv6_addr_any(&e.ip.in6))
> return -IPSET_ERR_HASH_ELEM;
>
> @@ -286,7 +287,8 @@ struct hash_ip6_elem {
> [IPSET_ATTR_PROBES] = { .type = NLA_U8 },
> [IPSET_ATTR_RESIZE] = { .type = NLA_U8 },
> [IPSET_ATTR_TIMEOUT]= { .type = NLA_U32 },
> - [IPSET_ATTR_NETMASK]= { .type = NLA_U8 },
> + [IPSET_ATTR_NETMASK]= { .type = NLA_U8 },
> + [IPSET_ATTR_NETMASK_MASK] = { .type = NLA_NESTED },
> [IPSET_ATTR_CADT_FLAGS] = { .type = NLA_U32 },
> },
> .adt_policy = {
> --
> 1.9.1
Best regards,
Jozsef
-
E-mail : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
H-1525 Budapest 114, POB. 49, Hungary
--
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
Re: [Outreachy kernel] Re: [PATCH] iptables: Constify option struct
On Tue, Mar 28, 2017 at 12:42 AM, Pablo Neira Ayuso wrote:
>
> On Tue, Mar 28, 2017 at 12:40:15AM +0530, Gargi Sharma wrote:
> > The struct of the type option is only used to initialise a field
> > inside the xtables_match struct and is not modified anywhere.
> > Done using Coccinelle:
> >
> > @r1 disable optional_qualifier@
> > identifier s,i;
> > position p;
> > @@
> >
> > static struct option i@p[] ={...};
> >
> > @ok1@
> > identifier r1.i;
> > expression e;
> > position p;
> > @@
> > e = i@p
> >
> > @bad@
> > position p != {r1.p,ok1.p};
> > identifier r1.i;
> > @@
> > e@i@p
> >
> > @depends on !bad disable optional_qualifier@
> > identifier r1.i;
> > @@
> > static
> > +const
> > struct option i[] = { ... };
>
> Applied, thanks.
>
> You can probably have a look at ebtables.git, probably there are more
> things to constify there.
>
Okay, I will. :)
Thanks!
Gargi
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/outreachy-kernel/20170327191239.GA18080%40salvia.
> For more options, visit https://groups.google.com/d/optout.
--
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
Re: [PATCH 2/4] netfilter: ipset: generalize netmask to support cidr and mask values
Hi Josh,
Overall, I like the feature and the patches. After reviewing I comment
only the parts where I believe some modifications are needed.
On Tue, 21 Mar 2017, Josh Hunt wrote:
> Extends ipset netmask support to handle both cidr values and full
> netmasks. As part of that it now supports wildcard masks allowing the
> user to mask out any bits of an address. This commit provides the
> infrastructure to specify netmasks of these types for hash sets of
> both v4 and v6 addressees.
>
> Follow on commits will add support for this type of netmasking to various
> hash set types.
>
> Signed-off-by: Josh Hunt
> ---
> include/linux/netfilter/ipset/ip_set.h | 3 +
> include/uapi/linux/netfilter/ipset/ip_set.h | 5 ++
> net/netfilter/ipset/ip_set_core.c | 2 +
> net/netfilter/ipset/ip_set_hash_gen.h | 91
> +
> 4 files changed, 89 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/netfilter/ipset/ip_set.h
> b/include/linux/netfilter/ipset/ip_set.h
> index 8e42253..0153cd3 100644
> --- a/include/linux/netfilter/ipset/ip_set.h
> +++ b/include/linux/netfilter/ipset/ip_set.h
> @@ -69,6 +69,7 @@ enum ip_set_extension {
> #define SET_WITH_COMMENT(s) ((s)->extensions & IPSET_EXT_COMMENT)
> #define SET_WITH_SKBINFO(s) ((s)->extensions & IPSET_EXT_SKBINFO)
> #define SET_WITH_FORCEADD(s) ((s)->flags & IPSET_CREATE_FLAG_FORCEADD)
> +#define SET_WITH_NETMASK(s) ((s)->flags & IPSET_CREATE_FLAG_NETMASK)
>
> /* Extension id, in size order */
> enum ip_set_ext_id {
> @@ -292,6 +293,8 @@ struct ip_set {
> cadt_flags |= IPSET_FLAG_WITH_SKBINFO;
> if (SET_WITH_FORCEADD(set))
> cadt_flags |= IPSET_FLAG_WITH_FORCEADD;
> + if (SET_WITH_NETMASK(set))
> + cadt_flags |= IPSET_FLAG_WITH_NETMASK;
>
> if (!cadt_flags)
> return 0;
> diff --git a/include/uapi/linux/netfilter/ipset/ip_set.h
> b/include/uapi/linux/netfilter/ipset/ip_set.h
> index ebb5154..2193908 100644
> --- a/include/uapi/linux/netfilter/ipset/ip_set.h
> +++ b/include/uapi/linux/netfilter/ipset/ip_set.h
> @@ -84,6 +84,7 @@ enum {
> IPSET_ATTR_CADT_LINENO = IPSET_ATTR_LINENO, /* 9 */
> IPSET_ATTR_MARK,/* 10 */
> IPSET_ATTR_MARKMASK,/* 11 */
> + IPSET_ATTR_NETMASK_MASK,/* 12 */
> /* Reserve empty slots */
> IPSET_ATTR_CADT_MAX = 16,
> /* Create-only specific attributes */
> @@ -200,6 +201,8 @@ enum ipset_cadt_flags {
> IPSET_FLAG_WITH_FORCEADD = (1 << IPSET_FLAG_BIT_WITH_FORCEADD),
> IPSET_FLAG_BIT_WITH_SKBINFO = 6,
> IPSET_FLAG_WITH_SKBINFO = (1 << IPSET_FLAG_BIT_WITH_SKBINFO),
> + IPSET_FLAG_BIT_WITH_NETMASK = 7,
> + IPSET_FLAG_WITH_NETMASK = (1 << IPSET_FLAG_BIT_WITH_NETMASK),
> IPSET_FLAG_CADT_MAX = 15,
> };
>
> @@ -207,6 +210,8 @@ enum ipset_cadt_flags {
> enum ipset_create_flags {
> IPSET_CREATE_FLAG_BIT_FORCEADD = 0,
> IPSET_CREATE_FLAG_FORCEADD = (1 << IPSET_CREATE_FLAG_BIT_FORCEADD),
> + IPSET_CREATE_FLAG_BIT_NETMASK = 1,
> + IPSET_CREATE_FLAG_NETMASK = (1 << IPSET_CREATE_FLAG_BIT_NETMASK),
> IPSET_CREATE_FLAG_BIT_MAX = 7,
> };
>
> diff --git a/net/netfilter/ipset/ip_set_core.c
> b/net/netfilter/ipset/ip_set_core.c
> index c296f9b..e2ce7ab 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -374,6 +374,8 @@ static inline struct ip_set_net *ip_set_pernet(struct net
> *net)
> cadt_flags = ip_set_get_h32(tb[IPSET_ATTR_CADT_FLAGS]);
> if (cadt_flags & IPSET_FLAG_WITH_FORCEADD)
> set->flags |= IPSET_CREATE_FLAG_FORCEADD;
> + if (cadt_flags & IPSET_FLAG_WITH_NETMASK)
> + set->flags |= IPSET_CREATE_FLAG_NETMASK;
> if (!align)
> align = 1;
> for (id = 0; id < IPSET_EXT_ID_MAX; id++) {
> diff --git a/net/netfilter/ipset/ip_set_hash_gen.h
> b/net/netfilter/ipset/ip_set_hash_gen.h
> index f236c0b..a407f85 100644
> --- a/net/netfilter/ipset/ip_set_hash_gen.h
> +++ b/net/netfilter/ipset/ip_set_hash_gen.h
> @@ -166,6 +166,41 @@ struct net_prefixes {
> #define NLEN 0
> #endif /* IP_SET_HASH_WITH_NETS */
>
> +#ifdef IP_SET_HASH_WITH_NETMASK
> +const static union nf_inet_addr onesmask = {
> + .all[0] = 0x,
> + .all[1] = 0x,
> + .all[2] = 0x,
> + .all[3] = 0x
> +};
> +const static union nf_inet_addr zeromask;
> +
> +struct ipset_netmask {
> + u8 cidr;
> + union nf_inet_addr mask;
> +};
> +
> +static void
> +ip_set_cidr_to_mask(union nf_inet_addr *addr, uint8_t cidr, uint8_t family)
> +{
> + uint8_t i;
> + uint8_t addrsize = (family == NFPROTO_IPV4) ? 1 : 4;
> +
> + for (i=0; i < addrsize; i++) {
> + if (!cidr) {
> + addr->all[i] = 0;
> + } else if (cidr >= 32) {
> + addr->all[i] = 0x;
> + cidr -= 32;
> +
Re: [PATCH] iptables: Constify option struct
On Tue, Mar 28, 2017 at 12:40:15AM +0530, Gargi Sharma wrote:
> The struct of the type option is only used to initialise a field
> inside the xtables_match struct and is not modified anywhere.
> Done using Coccinelle:
>
> @r1 disable optional_qualifier@
> identifier s,i;
> position p;
> @@
>
> static struct option i@p[] ={...};
>
> @ok1@
> identifier r1.i;
> expression e;
> position p;
> @@
> e = i@p
>
> @bad@
> position p != {r1.p,ok1.p};
> identifier r1.i;
> @@
> e@i@p
>
> @depends on !bad disable optional_qualifier@
> identifier r1.i;
> @@
> static
> +const
> struct option i[] = { ... };
Applied, thanks.
You can probably have a look at ebtables.git, probably there are more
things to constify there.
--
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
[PATCH] iptables: Constify option struct
The struct of the type option is only used to initialise a field
inside the xtables_match struct and is not modified anywhere.
Done using Coccinelle:
@r1 disable optional_qualifier@
identifier s,i;
position p;
@@
static struct option i@p[] ={...};
@ok1@
identifier r1.i;
expression e;
position p;
@@
e = i@p
@bad@
position p != {r1.p,ok1.p};
identifier r1.i;
@@
e@i@p
@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct option i[] = { ... };
Signed-off-by: Gargi Sharma
---
extensions/libebt_limit.c | 2 +-
extensions/libebt_mark.c | 2 +-
extensions/libebt_mark_m.c | 2 +-
extensions/libebt_nflog.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/extensions/libebt_limit.c b/extensions/libebt_limit.c
index 6b9bb16..988f678 100644
--- a/extensions/libebt_limit.c
+++ b/extensions/libebt_limit.c
@@ -29,7 +29,7 @@
#define ARG_LIMIT '1'
#define ARG_LIMIT_BURST'2'
-static struct option brlimit_opts[] =
+static const struct option brlimit_opts[] =
{
{ .name = "limit", .has_arg = true,.val = ARG_LIMIT },
{ .name = "limit-burst",.has_arg = true,.val = ARG_LIMIT_BURST
},
diff --git a/extensions/libebt_mark.c b/extensions/libebt_mark.c
index a1a208c..7b80b22 100644
--- a/extensions/libebt_mark.c
+++ b/extensions/libebt_mark.c
@@ -25,7 +25,7 @@ static int mark_supplied;
#define MARK_ORMARK '3'
#define MARK_ANDMARK '4'
#define MARK_XORMARK '5'
-static struct option brmark_opts[] = {
+static const struct option brmark_opts[] = {
{ .name = "mark-target",.has_arg = true,.val = MARK_TARGET },
/* an oldtime messup, we should have always used the scheme
* - */
diff --git a/extensions/libebt_mark_m.c b/extensions/libebt_mark_m.c
index ab9d234..eb08dba 100644
--- a/extensions/libebt_mark_m.c
+++ b/extensions/libebt_mark_m.c
@@ -18,7 +18,7 @@
#define MARK '1'
-static struct option brmark_m_opts[] = {
+static const struct option brmark_m_opts[] = {
{ .name = "mark", .has_arg = true, .val = MARK },
XT_GETOPT_TABLEEND,
};
diff --git a/extensions/libebt_nflog.c b/extensions/libebt_nflog.c
index fef7196..5f1d13b 100644
--- a/extensions/libebt_nflog.c
+++ b/extensions/libebt_nflog.c
@@ -30,7 +30,7 @@ enum {
NFLOG_NFLOG = 0x16,
};
-static struct option brnflog_opts[] = {
+static const struct option brnflog_opts[] = {
{ .name = "nflog-group", .has_arg = true, .val = NFLOG_GROUP},
{ .name = "nflog-prefix",.has_arg = true, .val = NFLOG_PREFIX},
{ .name = "nflog-range", .has_arg = true, .val = NFLOG_RANGE},
--
2.7.4
--
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
Re: [Outreachy kernel] [PATCH 0/4] Constify option structures
On Tue, Mar 28, 2017 at 12:15:26AM +0530, Gargi Sharma wrote:
> The struct option is generally used only to initialise
> fields inside certain structs and is not modified anywhere.
> Done using Coccinelle:
>
> @r1 disable optional_qualifier@
> identifier s,i;
> position p;
> @@
>
> static struct option i@p[] ={...};
>
> @ok1@
> identifier r1.i;
> expression e;
> position p;
> @@
> e = i@p
>
> @bad@
> position p != {r1.p,ok1.p};
> identifier r1.i;
> @@
> e@i@p
>
> @depends on !bad disable optional_qualifier@
> identifier r1.i;
> @@
> static
> +const
> struct option i[] = { ... };
Please indicate what userspace tree in the subject, I guess this is
iptables.
And merge them all into one single patch, they are part of the same
logical change.
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
[PATCH 3/4] extensions: libebt_mark: Constify option struct
The struct brmark_opts of the type option is only
used to initialise the extra_opts field in xtables_target
struct and is no modified anywhere.
Signed-off-by: Gargi Sharma
---
extensions/libebt_mark.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/extensions/libebt_mark.c b/extensions/libebt_mark.c
index a1a208c..7b80b22 100644
--- a/extensions/libebt_mark.c
+++ b/extensions/libebt_mark.c
@@ -25,7 +25,7 @@ static int mark_supplied;
#define MARK_ORMARK '3'
#define MARK_ANDMARK '4'
#define MARK_XORMARK '5'
-static struct option brmark_opts[] = {
+static const struct option brmark_opts[] = {
{ .name = "mark-target",.has_arg = true,.val = MARK_TARGET },
/* an oldtime messup, we should have always used the scheme
* - */
--
2.7.4
--
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
[PATCH 4/4] extensions: libebt_nflog: Constify option struct
The struct brnflog_opts of the type options is
used to initialise extra_opts field inside the
xtables_target struct and is not modified anywhere.
Signed-off-by: Gargi Sharma
---
extensions/libebt_nflog.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/extensions/libebt_nflog.c b/extensions/libebt_nflog.c
index fef7196..5f1d13b 100644
--- a/extensions/libebt_nflog.c
+++ b/extensions/libebt_nflog.c
@@ -30,7 +30,7 @@ enum {
NFLOG_NFLOG = 0x16,
};
-static struct option brnflog_opts[] = {
+static const struct option brnflog_opts[] = {
{ .name = "nflog-group", .has_arg = true, .val = NFLOG_GROUP},
{ .name = "nflog-prefix",.has_arg = true, .val = NFLOG_PREFIX},
{ .name = "nflog-range", .has_arg = true, .val = NFLOG_RANGE},
--
2.7.4
--
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
[PATCH 1/4] extensions: libebt_limit: Constify option struct
The struct brlimit_opts of the type option is only used to
initialise a field inside the xtables_match struct and is
not modified anywhere.
Signed-off-by: Gargi Sharma
---
extensions/libebt_limit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/extensions/libebt_limit.c b/extensions/libebt_limit.c
index 6b9bb16..988f678 100644
--- a/extensions/libebt_limit.c
+++ b/extensions/libebt_limit.c
@@ -29,7 +29,7 @@
#define ARG_LIMIT '1'
#define ARG_LIMIT_BURST'2'
-static struct option brlimit_opts[] =
+static const struct option brlimit_opts[] =
{
{ .name = "limit", .has_arg = true,.val = ARG_LIMIT },
{ .name = "limit-burst",.has_arg = true,.val = ARG_LIMIT_BURST
},
--
2.7.4
--
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
[PATCH 2/4] extenstions: libebt_mark_m: Constify option struct
The struct brmark_m_opts of the type option is only
used to initialise extra_opts field inside the xtables_match
struct and is not modified anywhere.
Signed-off-by: Gargi Sharma
---
extensions/libebt_mark_m.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/extensions/libebt_mark_m.c b/extensions/libebt_mark_m.c
index ab9d234..eb08dba 100644
--- a/extensions/libebt_mark_m.c
+++ b/extensions/libebt_mark_m.c
@@ -18,7 +18,7 @@
#define MARK '1'
-static struct option brmark_m_opts[] = {
+static const struct option brmark_m_opts[] = {
{ .name = "mark", .has_arg = true, .val = MARK },
XT_GETOPT_TABLEEND,
};
--
2.7.4
--
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
[PATCH 0/4] Constify option structures
The struct option is generally used only to initialise
fields inside certain structs and is not modified anywhere.
Done using Coccinelle:
@r1 disable optional_qualifier@
identifier s,i;
position p;
@@
static struct option i@p[] ={...};
@ok1@
identifier r1.i;
expression e;
position p;
@@
e = i@p
@bad@
position p != {r1.p,ok1.p};
identifier r1.i;
@@
e@i@p
@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct option i[] = { ... };
Gargi Sharma (4):
extensions: libebt_limit: Constify option struct
extenstions: libebt_mark_m: Constify option struct
extensions: libebt_mark: Constify option struct
extensions: libebt_nflog: Constify option struct
extensions/libebt_limit.c | 2 +-
extensions/libebt_mark.c | 2 +-
extensions/libebt_mark_m.c | 2 +-
extensions/libebt_nflog.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
--
2.7.4
--
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
[PATCH v2 ulogd2 2/2] rotate all default output files
Signed-off-by: Kaarle Ritvanen
---
ulogd.logrotate | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ulogd.logrotate b/ulogd.logrotate
index b3fb6d1..8470811 100644
--- a/ulogd.logrotate
+++ b/ulogd.logrotate
@@ -1,4 +1,4 @@
-/var/log/ulogd.log /var/log/ulogd.syslogemu /var/log/ulogd.pktlog
/var/log/ulogd.pcap {
+/var/log/ulogd* {
missingok
sharedscripts
postrotate
--
2.9.3
--
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
[PATCH v2 ulogd2 1/2] harmonize log file defaults with ulogd.conf
Signed-off-by: Kaarle Ritvanen
---
doc/ulogd.sgml | 4 ++--
output/ulogd_output_GPRINT.c | 2 +-
output/ulogd_output_LOGEMU.c | 2 +-
output/ulogd_output_NACCT.c | 2 +-
output/ulogd_output_OPRINT.c | 2 +-
5 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/doc/ulogd.sgml b/doc/ulogd.sgml
index 65a63cf..de799f4 100644
--- a/doc/ulogd.sgml
+++ b/doc/ulogd.sgml
@@ -422,7 +422,7 @@ to a file. The only useful application is debugging.
dumpfile
The filename where it should log to. The default is
-/var/log/ulogd.pktlog
+/var/log/ulogd_oprint.log
ulogd_output_LOGEMU.so
@@ -433,7 +433,7 @@ as possible. Logging is done to a seperate textfile instead
of syslog, though.
The module defines the following configuration directives:
fileThe filename where it should log to. The default is
-/var/log/ulogd.syslogemu
+/var/log/ulogd_syslogemu.log
syncSet this to 1 if you want to have your logfile written
synchronously. This may reduce performance, but makes your log-lines appear
immediately. The default is 0
diff --git a/output/ulogd_output_GPRINT.c b/output/ulogd_output_GPRINT.c
index 4f49c34..bc7aa34 100644
--- a/output/ulogd_output_GPRINT.c
+++ b/output/ulogd_output_GPRINT.c
@@ -31,7 +31,7 @@
#include
#ifndef ULOGD_GPRINT_DEFAULT
-#define ULOGD_GPRINT_DEFAULT "/var/log/ulogd.gprint"
+#define ULOGD_GPRINT_DEFAULT "/var/log/ulogd_gprint.log"
#endif
struct gprint_priv {
diff --git a/output/ulogd_output_LOGEMU.c b/output/ulogd_output_LOGEMU.c
index bb27209..cfcfab7 100644
--- a/output/ulogd_output_LOGEMU.c
+++ b/output/ulogd_output_LOGEMU.c
@@ -37,7 +37,7 @@
#endif
#ifndef ULOGD_LOGEMU_DEFAULT
-#define ULOGD_LOGEMU_DEFAULT "/var/log/ulogd.syslogemu"
+#define ULOGD_LOGEMU_DEFAULT "/var/log/ulogd_syslogemu.log"
#endif
#ifndef ULOGD_LOGEMU_SYNC_DEFAULT
diff --git a/output/ulogd_output_NACCT.c b/output/ulogd_output_NACCT.c
index 9de8a73..d369c7a 100644
--- a/output/ulogd_output_NACCT.c
+++ b/output/ulogd_output_NACCT.c
@@ -29,7 +29,7 @@
#include
#include
-#define NACCT_FILE_DEFAULT "/var/log/nacctdata.log"
+#define NACCT_FILE_DEFAULT "/var/log/ulogd_nacct.log"
/* config accessors (lazy me...) */
#define NACCT_CFG_FILE(pi) ((pi)->config_kset->ces[0].u.string)
diff --git a/output/ulogd_output_OPRINT.c b/output/ulogd_output_OPRINT.c
index e390c84..6fde445 100644
--- a/output/ulogd_output_OPRINT.c
+++ b/output/ulogd_output_OPRINT.c
@@ -28,7 +28,7 @@
#include
#ifndef ULOGD_OPRINT_DEFAULT
-#define ULOGD_OPRINT_DEFAULT "/var/log/ulogd.pktlog"
+#define ULOGD_OPRINT_DEFAULT "/var/log/ulogd_oprint.log"
#endif
#define NIPQUAD(addr) \
--
2.9.3
--
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
[PATCH] netfilter: nat: remove rcu_read_lock in __nf_nat_decode_session.
__nf_nat_decode_session is called from nf_nat_decode_session as decodefn. before calling decodefn, it already set rcu_read_lock. so rcu_read_lock in __nf_nat_decode_session can be removed. Signed-off-by: Taehee Yoo --- net/netfilter/nf_nat_core.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c index 94b14c5..9b68676 100644 --- a/net/netfilter/nf_nat_core.c +++ b/net/netfilter/nf_nat_core.c @@ -71,11 +71,10 @@ static void __nf_nat_decode_session(struct sk_buff *skb, struct flowi *fl) if (ct == NULL) return; - family = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.l3num; - rcu_read_lock(); + family = nf_ct_l3num(ct); l3proto = __nf_nat_l3proto_find(family); if (l3proto == NULL) - goto out; + return; dir = CTINFO2DIR(ctinfo); if (dir == IP_CT_DIR_ORIGINAL) @@ -84,8 +83,6 @@ static void __nf_nat_decode_session(struct sk_buff *skb, struct flowi *fl) statusbit = IPS_SRC_NAT; l3proto->decode_session(skb, ct, dir, statusbit, fl); -out: - rcu_read_unlock(); } int nf_xfrm_me_harder(struct net *net, struct sk_buff *skb, unsigned int family) -- 2.9.3 -- 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
[PATCH nf-next v2 1/1] netfilter: nat: Correct the return value check for nat mangled packet
From: Gao Feng
The return value type of function nf_nat_mangle_tcp/udp_packet is
int, but actually it is used as bool type. And most codes follow
this rule, for example, the sip, h323, and ftp. But some codes treat
the return value as NF_ACCEPT and NF_DROP, like amanda and irc.
Now use the bool type instead of the int to make it clear.
Signed-off-by: Gao Feng
---
v2: Merge two patches into one, and enhance the subject, per Pablo
v1: init version
include/net/netfilter/nf_nat_helper.h | 14 ++--
net/ipv4/netfilter/nf_nat_pptp.c | 20 +-
net/netfilter/nf_nat_amanda.c | 11 +-
net/netfilter/nf_nat_helper.c | 40 +--
net/netfilter/nf_nat_irc.c| 9
5 files changed, 46 insertions(+), 48 deletions(-)
diff --git a/include/net/netfilter/nf_nat_helper.h
b/include/net/netfilter/nf_nat_helper.h
index 01bcc6b..b27c391 100644
--- a/include/net/netfilter/nf_nat_helper.h
+++ b/include/net/netfilter/nf_nat_helper.h
@@ -7,13 +7,13 @@
struct sk_buff;
/* These return true or false. */
-int __nf_nat_mangle_tcp_packet(struct sk_buff *skb, struct nf_conn *ct,
+bool __nf_nat_mangle_tcp_packet(struct sk_buff *skb, struct nf_conn *ct,
enum ip_conntrack_info ctinfo,
unsigned int protoff, unsigned int match_offset,
unsigned int match_len, const char *rep_buffer,
unsigned int rep_len, bool adjust);
-static inline int nf_nat_mangle_tcp_packet(struct sk_buff *skb,
+static inline bool nf_nat_mangle_tcp_packet(struct sk_buff *skb,
struct nf_conn *ct,
enum ip_conntrack_info ctinfo,
unsigned int protoff,
@@ -27,11 +27,11 @@ static inline int nf_nat_mangle_tcp_packet(struct sk_buff
*skb,
rep_buffer, rep_len, true);
}
-int nf_nat_mangle_udp_packet(struct sk_buff *skb, struct nf_conn *ct,
-enum ip_conntrack_info ctinfo,
-unsigned int protoff, unsigned int match_offset,
-unsigned int match_len, const char *rep_buffer,
-unsigned int rep_len);
+bool nf_nat_mangle_udp_packet(struct sk_buff *skb, struct nf_conn *ct,
+ enum ip_conntrack_info ctinfo,
+ unsigned int protoff, unsigned int match_offset,
+ unsigned int match_len, const char *rep_buffer,
+ unsigned int rep_len);
/* Setup NAT on this expected conntrack so it follows master, but goes
* to port ct->master->saved_proto. */
diff --git a/net/ipv4/netfilter/nf_nat_pptp.c b/net/ipv4/netfilter/nf_nat_pptp.c
index b3ca21b..211fee5 100644
--- a/net/ipv4/netfilter/nf_nat_pptp.c
+++ b/net/ipv4/netfilter/nf_nat_pptp.c
@@ -177,11 +177,11 @@ pptp_outbound_pkt(struct sk_buff *skb,
ntohs(REQ_CID(pptpReq, cid_off)), ntohs(new_callid));
/* mangle packet */
- if (nf_nat_mangle_tcp_packet(skb, ct, ctinfo, protoff,
-cid_off + sizeof(struct pptp_pkt_hdr) +
-sizeof(struct PptpControlHeader),
-sizeof(new_callid), (char *)&new_callid,
-sizeof(new_callid)) == 0)
+ if (!nf_nat_mangle_tcp_packet(skb, ct, ctinfo, protoff,
+ cid_off + sizeof(struct pptp_pkt_hdr) +
+ sizeof(struct PptpControlHeader),
+ sizeof(new_callid), (char *)&new_callid,
+ sizeof(new_callid)))
return NF_DROP;
return NF_ACCEPT;
}
@@ -271,11 +271,11 @@ pptp_inbound_pkt(struct sk_buff *skb,
pr_debug("altering peer call id from 0x%04x to 0x%04x\n",
ntohs(REQ_CID(pptpReq, pcid_off)), ntohs(new_pcid));
- if (nf_nat_mangle_tcp_packet(skb, ct, ctinfo, protoff,
-pcid_off + sizeof(struct pptp_pkt_hdr) +
-sizeof(struct PptpControlHeader),
-sizeof(new_pcid), (char *)&new_pcid,
-sizeof(new_pcid)) == 0)
+ if (!nf_nat_mangle_tcp_packet(skb, ct, ctinfo, protoff,
+ pcid_off + sizeof(struct pptp_pkt_hdr) +
+ sizeof(struct PptpControlHeader),
+ sizeof(new_pcid), (char *)&new_pcid,
+ sizeof(new_pcid)))
return NF_DROP;
return NF_ACCEPT;
}
diff --git a/net/netfilter/nf_nat_amanda.c b/net/netfilter/nf_nat_amanda.c
index eb77238..e4d61a7 100644
RE: [PATCH] net: netfilter: Remove multiple assignment.
From: Pablo Neira Ayuso > Sent: 27 March 2017 13:08 > On Sat, Mar 25, 2017 at 06:19:47PM +0530, Arushi Singhal wrote: > > This patch removes multiple assignments. > > Done using coccinelle. > > @@ > > identifier i1,i2; > > constant c; > > @@ > > - i1=i2=c; > > + i1=c; > > + i2=c; > > You have to explain why this is bad. And your substituted code isn't equivalent. The correct replacement is: i2 = c; i1 = i2; David -- 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
RE: [PATCH nf v4 1/1] netfilter: snmp: Fix one possible panic when snmp_trap_helper fail to register
> -Original Message-
> From: netfilter-devel-ow...@vger.kernel.org
> [mailto:netfilter-devel-ow...@vger.kernel.org] On Behalf Of kbuild test
robot
> Sent: Monday, March 27, 2017 10:01 PM
> To: gfree.w...@foxmail.com
> Cc: kbuild-...@01.org; pa...@netfilter.org;
netfilter-devel@vger.kernel.org;
> gfree.w...@foxmail.com; Gao Feng
> Subject: Re: [PATCH nf v4 1/1] netfilter: snmp: Fix one possible panic
when
> snmp_trap_helper fail to register
>
> Hi Gao,
>
> [auto build test WARNING on nf/master]
>
> url:
>
https://github.com/0day-ci/linux/commits/gfree-wind-foxmail-com/netfilter-s
> nmp-Fix-one-possible-panic-when-snmp_trap_helper-fail-to-register/2017032
> 7-081220
> base: https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git
master
> config: x86_64-randconfig-s2-03272042 (attached as .config)
> compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All warnings (new ones prefixed by >>):
>
>net/ipv4//netfilter/nf_nat_snmp_basic.c: In function
> 'nf_nat_snmp_basic_init':
> >> net/ipv4//netfilter/nf_nat_snmp_basic.c:1281: warning: unused variable
> 'ret'
>
> vim +/ret +1281 net/ipv4//netfilter/nf_nat_snmp_basic.c
>
> 807467c22 Patrick McHardy 2006-12-02 1265 .help
=
> help,
> 6002f266b Patrick McHardy 2008-03-25 1266 .expect_policy
=
> &snmp_exp_policy,
> 807467c22 Patrick McHardy 2006-12-02 1267 .name
=
> "snmp_trap",
> 807467c22 Patrick McHardy 2006-12-02 1268 .tuple.src.l3num
=
> AF_INET,
> 09640e636 Harvey Harrison 2009-02-01 1269
.tuple.src.u.udp.port =
> cpu_to_be16(SNMP_TRAP_PORT),
> 807467c22 Patrick McHardy 2006-12-02 1270 .tuple.dst.protonum
=
> IPPROTO_UDP,
> 807467c22 Patrick McHardy 2006-12-02 1271 };
> 807467c22 Patrick McHardy 2006-12-02 1272
> 807467c22 Patrick McHardy 2006-12-02 1273
> /***
> **
> 807467c22 Patrick McHardy 2006-12-02 1274 *
> 807467c22 Patrick McHardy 2006-12-02 1275 * Module stuff.
> 807467c22 Patrick McHardy 2006-12-02 1276 *
> 807467c22 Patrick McHardy 2006-12-02 1277
>
> */
> 807467c22 Patrick McHardy 2006-12-02 1278
> 807467c22 Patrick McHardy 2006-12-02 1279 static int __init
> nf_nat_snmp_basic_init(void)
> 807467c22 Patrick McHardy 2006-12-02 1280 {
> 807467c22 Patrick McHardy 2006-12-02 @1281 int ret = 0;
> 807467c22 Patrick McHardy 2006-12-02 1282
> 93557f53e Jiri Olsa 2011-01-18 1283
> BUG_ON(nf_nat_snmp_hook != NULL);
> a9b3cd7f3 Stephen Hemminger 2011-08-01 1284
> RCU_INIT_POINTER(nf_nat_snmp_hook, help);
> 93557f53e Jiri Olsa 2011-01-18 1285
> 9b25a2987 Gao Feng 2017-03-25 1286 return
> nf_conntrack_helper_register(&snmp_trap_helper);
> 807467c22 Patrick McHardy 2006-12-02 1287 }
> 807467c22 Patrick McHardy 2006-12-02 1288
> 807467c22 Patrick McHardy 2006-12-02 1289 static void __exit
> nf_nat_snmp_basic_fini(void)
>
> :: The code at line 1281 was first introduced by commit
> :: 807467c22a12381985878be23c778657fc0364d0 [NETFILTER]: nf_nat: add
> SNMP NAT helper port
>
> :: TO: Patrick McHardy
> :: CC: David S. Miller
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology
> Center
> https://lists.01.org/pipermail/kbuild-all Intel
> Corporation
Thanks.
It is fixed in v5 patch.
Regards
Feng
--
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
Re: [PATCH nf v4 1/1] netfilter: snmp: Fix one possible panic when snmp_trap_helper fail to register
Hi Gao,
[auto build test WARNING on nf/master]
url:
https://github.com/0day-ci/linux/commits/gfree-wind-foxmail-com/netfilter-snmp-Fix-one-possible-panic-when-snmp_trap_helper-fail-to-register/20170327-081220
base: https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master
config: x86_64-randconfig-s2-03272042 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
net/ipv4//netfilter/nf_nat_snmp_basic.c: In function
'nf_nat_snmp_basic_init':
>> net/ipv4//netfilter/nf_nat_snmp_basic.c:1281: warning: unused variable 'ret'
vim +/ret +1281 net/ipv4//netfilter/nf_nat_snmp_basic.c
807467c22 Patrick McHardy 2006-12-02 1265.help = help,
6002f266b Patrick McHardy 2008-03-25 1266.expect_policy =
&snmp_exp_policy,
807467c22 Patrick McHardy 2006-12-02 1267.name =
"snmp_trap",
807467c22 Patrick McHardy 2006-12-02 1268.tuple.src.l3num=
AF_INET,
09640e636 Harvey Harrison 2009-02-01 1269.tuple.src.u.udp.port =
cpu_to_be16(SNMP_TRAP_PORT),
807467c22 Patrick McHardy 2006-12-02 1270.tuple.dst.protonum =
IPPROTO_UDP,
807467c22 Patrick McHardy 2006-12-02 1271 };
807467c22 Patrick McHardy 2006-12-02 1272
807467c22 Patrick McHardy 2006-12-02 1273
/*
807467c22 Patrick McHardy 2006-12-02 1274 *
807467c22 Patrick McHardy 2006-12-02 1275 * Module stuff.
807467c22 Patrick McHardy 2006-12-02 1276 *
807467c22 Patrick McHardy 2006-12-02 1277
*/
807467c22 Patrick McHardy 2006-12-02 1278
807467c22 Patrick McHardy 2006-12-02 1279 static int __init
nf_nat_snmp_basic_init(void)
807467c22 Patrick McHardy 2006-12-02 1280 {
807467c22 Patrick McHardy 2006-12-02 @1281int ret = 0;
807467c22 Patrick McHardy 2006-12-02 1282
93557f53e Jiri Olsa 2011-01-18 1283BUG_ON(nf_nat_snmp_hook !=
NULL);
a9b3cd7f3 Stephen Hemminger 2011-08-01 1284
RCU_INIT_POINTER(nf_nat_snmp_hook, help);
93557f53e Jiri Olsa 2011-01-18 1285
9b25a2987 Gao Feng 2017-03-25 1286return
nf_conntrack_helper_register(&snmp_trap_helper);
807467c22 Patrick McHardy 2006-12-02 1287 }
807467c22 Patrick McHardy 2006-12-02 1288
807467c22 Patrick McHardy 2006-12-02 1289 static void __exit
nf_nat_snmp_basic_fini(void)
:: The code at line 1281 was first introduced by commit
:: 807467c22a12381985878be23c778657fc0364d0 [NETFILTER]: nf_nat: add SNMP
NAT helper port
:: TO: Patrick McHardy
:: CC: David S. Miller
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
RE: [PATCH nf-next 1/1] netfilter: Use bool type instead of int as the return value of nf_conntrack_tuple_taken and nf_nat_used_tuple
> -Original Message- > From: netfilter-devel-ow...@vger.kernel.org > [mailto:netfilter-devel-ow...@vger.kernel.org] On Behalf Of Pablo Neira Ayuso > Sent: Monday, March 27, 2017 8:25 PM > To: f...@ikuai8.com > Cc: netfilter-devel@vger.kernel.org; gfree.w...@gmail.com > Subject: Re: [PATCH nf-next 1/1] netfilter: Use bool type instead of int as the > return value of nf_conntrack_tuple_taken and nf_nat_used_tuple > > On Tue, Mar 07, 2017 at 12:28:55PM +0800, f...@ikuai8.com wrote: > > From: Gao Feng > > > > These two functions return 1/0 as true or false, so it could use bool > > type directly instead of int. > > Too long patch subject. Ok, I would make it shorter. > > > Signed-off-by: Gao Feng > > --- > > include/net/netfilter/nf_conntrack.h | 2 +- > > include/net/netfilter/nf_nat.h | 2 +- > > net/netfilter/nf_conntrack_core.c| 6 +++--- > > net/netfilter/nf_nat_core.c | 2 +- > > 4 files changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/include/net/netfilter/nf_conntrack.h > > b/include/net/netfilter/nf_conntrack.h > > index f540f9a..32a0394 100644 > > --- a/include/net/netfilter/nf_conntrack.h > > +++ b/include/net/netfilter/nf_conntrack.h > > @@ -160,7 +160,7 @@ void nf_conntrack_alter_reply(struct nf_conn *ct, > > > > /* Is this tuple taken? (ignoring any belonging to the given > > conntrack). */ > > -int nf_conntrack_tuple_taken(const struct nf_conntrack_tuple *tuple, > > +bool nf_conntrack_tuple_taken(const struct nf_conntrack_tuple *tuple, > > const struct nf_conn *ignored_conntrack); > > Did you audit callers if they handle this type change accordingly? I don't see > any description about this on your patch. I will add the caller description in the update patch. Regards Feng > -- > 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 -- 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
RE: [PATCH nf-next 1/4] netfilter: amanda: Correct the return value comparison of the func nf_nat_mangle_udp_packet
> -Original Message-
> From: Pablo Neira Ayuso [mailto:pa...@netfilter.org]
> Sent: Monday, March 27, 2017 8:13 PM
> To: f...@ikuai8.com
> Cc: netfilter-devel@vger.kernel.org; gfree.w...@gmail.com
> Subject: Re: [PATCH nf-next 1/4] netfilter: amanda: Correct the return
value
> comparison of the func nf_nat_mangle_udp_packet
>
> On Fri, Mar 17, 2017 at 02:47:19PM +0800, f...@ikuai8.com wrote:
> > From: Gao Feng
> >
> > The return value of nf_nat_mangle_udp_packet actually is 1 and 0 as
> > bool type. But the amanda codes compare it with NF_ACCEPT.
> >
> > Signed-off-by: Gao Feng
> > ---
> > net/netfilter/nf_nat_amanda.c | 11 +--
> > 1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/netfilter/nf_nat_amanda.c
> > b/net/netfilter/nf_nat_amanda.c index eb77238..e4d61a7 100644
> > --- a/net/netfilter/nf_nat_amanda.c
> > +++ b/net/netfilter/nf_nat_amanda.c
> > @@ -33,7 +33,6 @@ static unsigned int help(struct sk_buff *skb, {
> > char buffer[sizeof("65535")];
> > u_int16_t port;
> > - unsigned int ret;
> >
> > /* Connection comes from client. */
> > exp->saved_proto.tcp.port = exp->tuple.dst.u.tcp.port; @@ -63,14
> > +62,14 @@ static unsigned int help(struct sk_buff *skb,
> > }
> >
> > sprintf(buffer, "%u", port);
> > - ret = nf_nat_mangle_udp_packet(skb, exp->master, ctinfo,
> > - protoff, matchoff, matchlen,
> > - buffer, strlen(buffer));
> > - if (ret != NF_ACCEPT) {
> > + if (!nf_nat_mangle_udp_packet(skb, exp->master, ctinfo,
> > + protoff, matchoff, matchlen,
> > + buffer, strlen(buffer))) {
> > nf_ct_helper_log(skb, exp->master, "cannot mangle packet");
> > nf_ct_unexpect_related(exp);
> > + return NF_DROP;
> > }
> > - return ret;
> > + return NF_ACCEPT;
>
> This cleanup patches are a bit oversplit.
>
> Better, send one patch where you update nf_nat_mangle_udp_packet() and
> nf_nat_mangle_tcp_packet() to return boolean and update *all of the
netfilter
> spots* where we use them accordingly.
>
> Please be careful on this...
Ok, I would merge them into one patch.
I already checked them before, but I would check it again after merge.
Regards
Feng
--
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
Re: [PATCH 0/2] netfilter: Remove unnecessary cast on void pointer
On Tue, Mar 21, 2017 at 05:49:52PM +0530, simran singhal wrote: > This patch series remove unnecessary cast on void pointer. > > simran singhal (2): > netfilter: ipset: Remove unnecessary cast on void pointer > netfilter: Remove unnecessary cast on void pointer Please, merge this two patches in one single patch. I have to pass up batches to David, and I would like this cleanups are grouped into the same logical change. Is there more occurrences of this in the Netfilter tree? Please have a look at net/ipv4/netfilter/ net/ipv6/netfilter/ and net/bridge/netfilter, we also have code there. 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
Re: [PATCH nf-next 1/1] netfilter: Use bool type instead of int as the return value of nf_conntrack_tuple_taken and nf_nat_used_tuple
On Tue, Mar 07, 2017 at 12:28:55PM +0800, f...@ikuai8.com wrote: > From: Gao Feng > > These two functions return 1/0 as true or false, so it could use bool > type directly instead of int. Too long patch subject. > Signed-off-by: Gao Feng > --- > include/net/netfilter/nf_conntrack.h | 2 +- > include/net/netfilter/nf_nat.h | 2 +- > net/netfilter/nf_conntrack_core.c| 6 +++--- > net/netfilter/nf_nat_core.c | 2 +- > 4 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/include/net/netfilter/nf_conntrack.h > b/include/net/netfilter/nf_conntrack.h > index f540f9a..32a0394 100644 > --- a/include/net/netfilter/nf_conntrack.h > +++ b/include/net/netfilter/nf_conntrack.h > @@ -160,7 +160,7 @@ void nf_conntrack_alter_reply(struct nf_conn *ct, > > /* Is this tuple taken? (ignoring any belonging to the given > conntrack). */ > -int nf_conntrack_tuple_taken(const struct nf_conntrack_tuple *tuple, > +bool nf_conntrack_tuple_taken(const struct nf_conntrack_tuple *tuple, >const struct nf_conn *ignored_conntrack); Did you audit callers if they handle this type change accordingly? I don't see any description about this on your patch. -- 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
Re: [Outreachy kernel] Re: [PATCH] net: netfilter: Remove multiple assignment.
On Mon, Mar 27, 2017 at 05:48:41PM +0530, Arushi Singhal wrote: > On Mon, Mar 27, 2017 at 5:38 PM, Pablo Neira Ayuso > wrote: > > > On Sat, Mar 25, 2017 at 06:19:47PM +0530, Arushi Singhal wrote: > > > This patch removes multiple assignments. > > > Done using coccinelle. > > > @@ > > > identifier i1,i2; > > > constant c; > > > @@ > > > - i1=i2=c; > > > + i1=c; > > > + i2=c; > > > > You have to explain why this is bad. > > > > It is against the kernel coding style and we have to avoid multiple > assignments to make the code more readable. > This error is found using Checkpatch.pl script. Then, please place this information in your patch description. 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
Re: [PATCH nf-next 1/4] netfilter: amanda: Correct the return value comparison of the func nf_nat_mangle_udp_packet
On Fri, Mar 17, 2017 at 02:47:19PM +0800, f...@ikuai8.com wrote:
> From: Gao Feng
>
> The return value of nf_nat_mangle_udp_packet actually is 1 and 0 as
> bool type. But the amanda codes compare it with NF_ACCEPT.
>
> Signed-off-by: Gao Feng
> ---
> net/netfilter/nf_nat_amanda.c | 11 +--
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/net/netfilter/nf_nat_amanda.c b/net/netfilter/nf_nat_amanda.c
> index eb77238..e4d61a7 100644
> --- a/net/netfilter/nf_nat_amanda.c
> +++ b/net/netfilter/nf_nat_amanda.c
> @@ -33,7 +33,6 @@ static unsigned int help(struct sk_buff *skb,
> {
> char buffer[sizeof("65535")];
> u_int16_t port;
> - unsigned int ret;
>
> /* Connection comes from client. */
> exp->saved_proto.tcp.port = exp->tuple.dst.u.tcp.port;
> @@ -63,14 +62,14 @@ static unsigned int help(struct sk_buff *skb,
> }
>
> sprintf(buffer, "%u", port);
> - ret = nf_nat_mangle_udp_packet(skb, exp->master, ctinfo,
> -protoff, matchoff, matchlen,
> -buffer, strlen(buffer));
> - if (ret != NF_ACCEPT) {
> + if (!nf_nat_mangle_udp_packet(skb, exp->master, ctinfo,
> + protoff, matchoff, matchlen,
> + buffer, strlen(buffer))) {
> nf_ct_helper_log(skb, exp->master, "cannot mangle packet");
> nf_ct_unexpect_related(exp);
> + return NF_DROP;
> }
> - return ret;
> + return NF_ACCEPT;
This cleanup patches are a bit oversplit.
Better, send one patch where you update nf_nat_mangle_udp_packet() and
nf_nat_mangle_tcp_packet() to return boolean and update *all of the
netfilter spots* where we use them accordingly.
Please be careful on this...
--
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
Re: [PATCH] net: netfilter: Remove multiple assignment.
On Sat, Mar 25, 2017 at 06:19:47PM +0530, Arushi Singhal wrote: > This patch removes multiple assignments. > Done using coccinelle. > @@ > identifier i1,i2; > constant c; > @@ > - i1=i2=c; > + i1=c; > + i2=c; You have to explain why this is bad. -- 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
Re: [PATCH] net: netfilters: Remove extra parenthesis
Hi Arushi,
On Sat, Mar 25, 2017 at 07:23:13PM +0530, Arushi Singhal wrote:
> diff --git a/net/netfilter/nf_conntrack_netlink.c
> b/net/netfilter/nf_conntrack_netlink.c
> index 6806b5e73567..aa344c5868c5 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -467,7 +467,7 @@ ctnetlink_fill_info(struct sk_buff *skb, u32 portid, u32
> seq, u32 type,
> struct nlattr *nest_parms;
> unsigned int flags = portid ? NLM_F_MULTI : 0, event;
>
> - event = (NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_NEW);
> + event = NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_NEW;
Could you send us a unfront patch to add something like:
static inline u16 nfnl_msg_type(u8 subsys, u8 msg_type)
{
return subsys << 8 | msg_type;
}
I would suggest you place this in include/linux/netfilter/nfnetlink.h
Then, use it here.
> nlh = nlmsg_put(skb, portid, seq, event, sizeof(*nfmsg), flags);
> if (nlh == NULL)
> goto nlmsg_failure;
> @@ -1983,7 +1983,7 @@ ctnetlink_ct_stat_cpu_fill_info(struct sk_buff *skb,
> u32 portid, u32 seq,
> struct nfgenmsg *nfmsg;
> unsigned int flags = portid ? NLM_F_MULTI : 0, event;
>
> - event = (NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_GET_STATS_CPU);
> + event = NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_GET_STATS_CPU;
... And here too.
> nlh = nlmsg_put(skb, portid, seq, event, sizeof(*nfmsg), flags);
> if (nlh == NULL)
> goto nlmsg_failure;
> @@ -2066,7 +2066,7 @@ ctnetlink_stat_ct_fill_info(struct sk_buff *skb, u32
> portid, u32 seq, u32 type,
> unsigned int flags = portid ? NLM_F_MULTI : 0, event;
> unsigned int nr_conntracks = atomic_read(&net->ct.count);
>
> - event = (NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_GET_STATS);
> + event = NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_GET_STATS;
And so on. Look for more spots where we can replace this opencoded
thing.
I guess there are more spots in all of the net/netfilter/ netlink
subsystems.
Once that patch gets in, you can follow up with this parens cleanup.
Thanks!
P.S: Cc'ing netfilter-devel@vger.kernel.org (and
lvs-de...@vger.kernel.org if you touch 'ipvs' bits) should be fine. No
need to Cc all those many lists, better to narrow down you target.
--
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
Re: [PATCH nf] netfilter: nf_ct_ext: fix possible panic after nf_ct_extend_unregister
On Sat, Mar 25, 2017 at 04:35:29PM +0800, Liping Zhang wrote: > From: Liping Zhang > > If one cpu is doing nf_ct_extend_unregister while another cpu is doing > __nf_ct_ext_add_length, then we may hit BUG_ON(t == NULL). Moreover, > there's no synchronize_rcu invocation after set nf_ct_ext_types[id] to > NULL, so it's possible that we may access invalid pointer. > > But actually, most of the ct extends are built-in, so the problem listed > above will not happen. However, there are two exceptions: NF_CT_EXT_NAT > and NF_CT_EXT_SYNPROXY. > > For _EXT_NAT, the panic will not happen, since adding the nat extend and > unregistering the nat extend are located in the same file(nf_nat_core.c), > this means that after the nat module is removed, we cannot add the nat > extend too. > > For _EXT_SYNPROXY, synproxy extend may be added by init_conntrack, while > synproxy extend unregister will be done by synproxy_core_exit. So after > nf_synproxy_core.ko is removed, we may still try to add the synproxy > extend, then kernel panic may happen. > > I know it's very hard to reproduce this issue, but I can play a tricky > game to make it happen very easily :) > > Step 1. Enable SYNPROXY for tcp dport 1234 at FORWARD hook: > # iptables -I FORWARD -p tcp --dport 1234 -j SYNPROXY > Step 2. Queue the syn packet to the userspace at raw table OUTPUT hook. > Also note, in the userspace we only add a 20s' delay, then > reinject the syn packet to the kernel: > # iptables -t raw -I OUTPUT -p tcp --syn -j NFQUEUE --queue-num 1 > Step 3. Using "nc 2.2.2.2 1234" to connect the server. > Step 4. Now remove the nf_synproxy_core.ko quickly: > # iptables -F FORWARD > # rmmod ipt_SYNPROXY > # rmmod nf_synproxy_core > Step 5. After 20s' delay, the syn packet is reinjected to the kernel. > > Now you will see the panic like this: > kernel BUG at net/netfilter/nf_conntrack_extend.c:91! > Call Trace: >? __nf_ct_ext_add_length+0x53/0x3c0 [nf_conntrack] >init_conntrack+0x12b/0x600 [nf_conntrack] >nf_conntrack_in+0x4cc/0x580 [nf_conntrack] >ipv4_conntrack_local+0x48/0x50 [nf_conntrack_ipv4] >nf_reinject+0x104/0x270 >nfqnl_recv_verdict+0x3e1/0x5f9 [nfnetlink_queue] >? nfqnl_recv_verdict+0x5/0x5f9 [nfnetlink_queue] >? nla_parse+0xa0/0x100 >nfnetlink_rcv_msg+0x175/0x6a9 [nfnetlink] >[...] > > One possible solution is to make NF_CT_EXT_SYNPROXY extend built-in, i.e. > introduce nf_conntrack_synproxy.c and only do ct extend register and > unregister in it, similar to nf_conntrack_timeout.c. > > But having such a obscure restriction of nf_ct_extend_unregister is not a > good idea, so we should invoke synchronize_rcu after set nf_ct_ext_types > to NULL, and check the NULL pointer when do __nf_ct_ext_add_length. Then > it will be easier if we add new ct extend in the future. > > Last, we use kfree_rcu to free nf_ct_ext, so rcu_barrier() is unnecessary > anymore, remove it too. Also applied, 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
Re: [PATCH nf V3] netfilter: nfnl_cthelper: fix a race when walk the nf_ct_helper_hash table
On Sat, Mar 25, 2017 at 12:09:15PM +0800, Liping Zhang wrote: > From: Liping Zhang > > The nf_ct_helper_hash table is protected by nf_ct_helper_mutex, while > nfct_helper operation is protected by nfnl_lock(NFNL_SUBSYS_CTHELPER). > So it's possible that one CPU is walking the nf_ct_helper_hash for > cthelper add/get/del, another cpu is doing nf_conntrack_helpers_unregister > at the same time. This is dangrous, and may cause use after free error. > > Note, delete operation will flush all cthelpers added via nfnetlink, so > using rcu to do protect is not easy. > > Now introduce a dummy list to record all the cthelpers added via > nfnetlink, then we can walk the dummy list instead of walking the > nf_ct_helper_hash. Also, keep nfnl_cthelper_dump_table unchanged, it > may be invoked without nfnl_lock(NFNL_SUBSYS_CTHELPER) held. Also applied, thanks Liping. -- 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
Re: [PATCH nf V2] netfilter: invoke synchronize_rcu after set the _hook_ to NULL
On Sat, Mar 25, 2017 at 08:53:12AM +0800, Liping Zhang wrote: > From: Liping Zhang > > Otherwise, another CPU may access the invalid pointer. For example: > CPU0CPU1 > - rcu_read_lock(); > - pfunc = _hook_; > _hook_ = NULL; - > mod unload - > - pfunc(); // invalid, panic > - rcu_read_unlock(); > > So we must call synchronize_rcu() to wait the rcu reader to finish. > > Also note, in nf_nat_snmp_basic_fini, synchronize_rcu() will be invoked > by later nf_conntrack_helper_unregister, but I'm inclined to add a > explicit synchronize_rcu after set the nf_nat_snmp_hook to NULL. Depend > on such obscure assumptions is not a good idea. > > Last, in nfnetlink_cttimeout, we use kfree_rcu to free the time object, > so in cttimeout_exit, invoking rcu_barrier() is not necessary at all, > remove it too. Applied, 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
Re: [PATCH nf v5 1/1] netfilter: snmp: Fix one possible panic when snmp_trap_helper fail to register
On Sat, Mar 25, 2017 at 06:24:36PM +0800, gfree.w...@foxmail.com wrote:
> From: Gao Feng
>
> In the commit 93557f53e1fb ("netfilter: nf_conntrack: nf_conntrack snmp
> helper"), the snmp_helper is replaced by nf_nat_snmp_hook. So the
> snmp_helper is never registered. But it still tries to unregister the
> snmp_helper, it could cause the panic.
>
> Now remove the useless snmp_helper and the unregister call in the
> error handler.
Applied to nf, 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
