Re: [PATCH iptables] extensions: libxt_hashlimit: fix 64-bit printf formats

2017-04-10 Thread James Cowgill
On 08/04/17 21:29, Pablo Neira Ayuso wrote:
> On Fri, Apr 07, 2017 at 12:47:29PM +0100, James Cowgill wrote:
>> hashlimit was using "%lu" in a lot of printf format specifiers to print
>> 64-bit integers. This is incorrect on 32-bit architectures because
>> "long int" is 32-bits there. On MIPS, it was causing iptables to
>> segfault when printing these integers.
>>
>> Fix by using the correct format specifier.
> 
> One comment below...
> 
>> @@ -262,7 +262,7 @@ static uint64_t parse_burst(const char *burst, int 
>> revision)
>>  if (v > max)
>>  xtables_error(PARAMETER_PROBLEM, "bad value for option "
>>  "\"--hashlimit-burst\", value \"%s\" too large "
>> -"(max %lumb).", burst, max/1024/1024);
>> +"(max %" PRIu64 "mb).", burst, max/1024/1024);
> ^  ^
> 
> I can remove this whitespaces, right? I'm looking at other spots in
> the iptables code and I would like to keep this consistent.
> 
> No need to resent the patch, I can just mangle this here and apply.

Yes removing the spaces around PRIu64 should be fine.

James
--
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 1/1] netfilter: seqadj: Fix possible non-linear data access for TCP header

2017-04-10 Thread gfree . wind
From: Gao Feng 

The current call path of nf_ct_tcp_seqadj_set is the following.

nfqnl_recv_verdict->ctnetlink_glue_hook->ctnetlink_glue_seqadj
->nf_ct_tcp_seqadj_set.

It couldn't make sure the TCP header is in the linear data part.
So use the skb_header_pointer instead of the current codes.

BTW, the nf_ct_tcp_seqadj_set is one external function of netfilter
which works in the network layer, it should not assume the transport
header is in the linear data.

Signed-off-by: Gao Feng 
---
 net/netfilter/nf_conntrack_seqadj.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_seqadj.c 
b/net/netfilter/nf_conntrack_seqadj.c
index ef7063e..80394ab 100644
--- a/net/netfilter/nf_conntrack_seqadj.c
+++ b/net/netfilter/nf_conntrack_seqadj.c
@@ -61,11 +61,14 @@ void nf_ct_tcp_seqadj_set(struct sk_buff *skb,
  s32 off)
 {
const struct tcphdr *th;
+   struct tcphdr tcph;
 
if (nf_ct_protonum(ct) != IPPROTO_TCP)
return;
 
-   th = (struct tcphdr *)(skb_network_header(skb) + ip_hdrlen(skb));
+   th = skb_header_pointer(skb, ip_hdrlen(skb), sizeof(tcph), &tcph);
+   if (!th)
+   return;
nf_ct_seqadj_set(ct, ctinfo, th->seq, off);
 }
 EXPORT_SYMBOL_GPL(nf_ct_tcp_seqadj_set);
-- 
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


Re: [PATCH] net: netfilter: Replace explicit NULL comparisons

2017-04-10 Thread Pablo Neira Ayuso
On Sun, Apr 09, 2017 at 09:12:18AM +0530, Arushi Singhal wrote:
> On Sun, Apr 9, 2017 at 1:44 AM, Pablo Neira Ayuso 
> wrote:
> 
> > On Sat, Apr 08, 2017 at 08:21:56PM +0200, Jan Engelhardt wrote:
> > > On Saturday 2017-04-08 19:21, Arushi Singhal wrote:
> > >
> > > >Replace explicit NULL comparison with ! operator to simplify code.
> > >
> > > I still wouldn't do this, for the same reason as before. Comparing to
> > > NULL explicitly more or less gave an extra guarantee that the other
> > > operand was also a pointer.
> >
> > Arushi, where does it say in the coding style that this is prefered?
> 
> This is reported by checkpatch.pl script.

I don't find it in the coding style. I think this is what it stands as
preference in this case IMO. Otherwise, it would be good to get the
kernel coding style document in sync with it, including the reason why
this way to express thing is cleaner. We have to justify the changes.
--
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: ipvs: Replace explicit NULL comparison

2017-04-10 Thread Pablo Neira Ayuso
Arushi,

On Sun, Apr 09, 2017 at 06:21:51AM +0800, kbuild test robot wrote:
> Hi Arushi,
> 
> [auto build test WARNING on ipvs-next/master]
> [also build test WARNING on v4.11-rc5 next-20170407]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Arushi-Singhal/net-netfilter-ipvs-Replace-explicit-NULL-comparison/20170409-044710
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-next.git 
> master
> config: i386-randconfig-x002-201715 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386 
> 
> All warnings (new ones prefixed by >>):
> 
>net/netfilter/ipvs/ip_vs_proto.c: In function 'ip_vs_protocol_net_cleanup':
> >> net/netfilter/ipvs/ip_vs_proto.c:350:3: warning: suggest parentheses 
> >> around assignment used as truth value [-Wparentheses]
>   while (pd = ipvs->proto_data_table[i])
>   ^

This is bad, you have to be more careful in what you do. This is not a
speed coding contest.

Showing careful patchset handling, even if you submit less of them, is
way more prefered in my opinion.
--
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] iptables-restore/ip6tables-restore: add --version/-V argument

2017-04-10 Thread Pablo Neira Ayuso
On Sat, Apr 08, 2017 at 10:10:16PM -0500, Dan Williams wrote:
> On Sun, 2017-04-09 at 00:04 +0200, Pablo Neira Ayuso wrote:
> > On Mon, Apr 03, 2017 at 04:32:40PM -0500, Dan Williams wrote:
> > > Prints program version just like iptables/ip6tables.
> > 
> > Applied, thanks.
> > 
> > You sent us a chunk for the manpage?
> 
> I did not, but I will.  I noticed there wasn't one for the iptables-
> restore --wait changes either, was Lorenzo going to send one?  Mine
> would probably conflict with that.

Right. I forgot to ask for that chunk to Lorenzo.

If you can help us on adding these missing chunks to the manpage, I
would really appreciate this. 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] netfilter: ctnetlink: remove unnecessary nf_conntrack_expect_lock protection

2017-04-10 Thread Pablo Neira Ayuso
On Sun, Apr 09, 2017 at 12:21:22PM +0800, Liping Zhang wrote:
> Hi Pablo,
> 
> 2017-04-09 5:16 GMT+08:00 Pablo Neira Ayuso :
> > On Sat, Apr 01, 2017 at 10:14:24PM +0800, Liping Zhang wrote:
> >> @@ -1960,9 +1955,7 @@ static int ctnetlink_new_conntrack(struct net *net, 
> >> struct sock *ctnl,
> >>   err = -EEXIST;
> >>   ct = nf_ct_tuplehash_to_ctrack(h);
> >>   if (!(nlh->nlmsg_flags & NLM_F_EXCL)) {
> >> - spin_lock_bh(&nf_conntrack_expect_lock);
> >>   err = ctnetlink_change_conntrack(ct, cda);
> >> - spin_unlock_bh(&nf_conntrack_expect_lock);
> >
> > We used to have a central spinlock here.
> >
> > spin_lock_bh(&nf_conntrack_lock);
> >
> > that was removed time ago, so this go converted to use
> > nf_conntrack_expect_lock.
> 
> This patch should add:
> 
> Fixes: ca7433df3a67 ("netfilter: conntrack: seperate expect locking
> from nf_conntrack_lock")
> 
> Commit ca7433df3a67 add spin_lock_bh(&nf_conntrack_expect_lock) in
> nf_ct_remove_expectations, but we also lock the _expect_lock before calling
> ctnetlink_change_conntrack, so dead lock will happen:
> 
>  spin_lock_bh(&nf_conntrack_expect_lock):
> ->err = ctnetlink_change_conntrack(ct, cda)
> -->ctnetlink_change_helper
> --->if (!strcmp(helpname, "")) nf_ct_remove_expectations()
> >spin_lock_bh(&nf_conntrack_expect_lock); //lock _expect_lock
> again, dead lock!

I agree this is fixing the deadlock but see below.

> Since ctnetlink_change_conntrack is unrelated to nf_conntrack_expect_lock,
> so remove it can fix this issue.

But packets may be updating a conntrack at the same time that we're
mangling via ctnetlink, right?
--
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 1/1] netfilter: seqadj: Fix possible non-linear data access for TCP header

2017-04-10 Thread Pablo Neira Ayuso
On Mon, Apr 10, 2017 at 06:36:03PM +0800, gfree.w...@foxmail.com wrote:
> From: Gao Feng 
> 
> The current call path of nf_ct_tcp_seqadj_set is the following.
> 
> nfqnl_recv_verdict->ctnetlink_glue_hook->ctnetlink_glue_seqadj
> ->nf_ct_tcp_seqadj_set.
> 
> It couldn't make sure the TCP header is in the linear data part.
> So use the skb_header_pointer instead of the current codes.
> 
> BTW, the nf_ct_tcp_seqadj_set is one external function of netfilter
> which works in the network layer, it should not assume the transport
> header is in the linear data.
> 
> Signed-off-by: Gao Feng 
> ---
>  net/netfilter/nf_conntrack_seqadj.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_conntrack_seqadj.c 
> b/net/netfilter/nf_conntrack_seqadj.c
> index ef7063e..80394ab 100644
> --- a/net/netfilter/nf_conntrack_seqadj.c
> +++ b/net/netfilter/nf_conntrack_seqadj.c
> @@ -61,11 +61,14 @@ void nf_ct_tcp_seqadj_set(struct sk_buff *skb,
> s32 off)
>  {
>   const struct tcphdr *th;
> + struct tcphdr tcph;
>  
>   if (nf_ct_protonum(ct) != IPPROTO_TCP)
>   return;
>  
> - th = (struct tcphdr *)(skb_network_header(skb) + ip_hdrlen(skb));
> + th = skb_header_pointer(skb, ip_hdrlen(skb), sizeof(tcph), &tcph);
> + if (!th)
> + return;

This fix is required since your recent upgrade to check for 4 bytes
instead of 8 bytes from conntrack. Please confirm this.

If so, it would be good to review the NAT code to see if there are
more spots that are not broken since that change...
--
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] netfilter: nat: remove rcu_read_lock in __nf_nat_decode_session.

2017-04-10 Thread Taehee Yoo
Thank you for your review!

2017-04-07 4:51 GMT+09:00 Pablo Neira Ayuso :
> On Tue, Mar 28, 2017 at 12:28:50AM +0900, Taehee Yoo wrote:
>> __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.
>
> Could you have close look at the tree to confirm if we have more spots
> where rcu_read_lock is unnecessary?
>
> $ git grep rcu_read_lock net/netfilter/ | wc -l
> 158
> $ git grep rcu_read_lock
> net/ipv4/netfilter/ | wc -l
> 3
> $ git grep rcu_read_lock
> net/ipv6/netfilter/ | wc -l
> 2
> $ git grep rcu_read_lock
> net/bridge/netfilter/ | wc -l
>
> It's a fair good amount, it would take me around one hour probably to
> evaluate those here.
>
> If you can help us on verifying this, it would be simply great.
>
> Thanks!

okay, I confirm whole rcu_read_lock then I will resend a single patch
as soon as possible.
may be it needs a week.
--
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: ctnetlink: remove unnecessary nf_conntrack_expect_lock protection

2017-04-10 Thread Liping Zhang
Hi Pablo,

2017-04-10 20:02 GMT+08:00 Pablo Neira Ayuso :
[...]
>> Since ctnetlink_change_conntrack is unrelated to nf_conntrack_expect_lock,
>> so remove it can fix this issue.
>
> But packets may be updating a conntrack at the same time that we're
> mangling via ctnetlink, right?

Yes, but in packets processing path, we use rcu_read_lock(), so using
spin_lock_bh(&nf_conntrack_expect_lock) here won't help anything.

As a quick summary(just a reference):
1. For CTA_TIMEOUT, there's no problem
2. For CTA_MARK, no problem too
3. For CTA_PROTOINFO, spin_lock_bh(&ct->lock) will be held, so no problem too
4. For CTA_LABELS, it may race with packets path, but it seems not a big problem
5. For CTA_SEQ_ADJ_ORIG... we should hold &ct->lock when do updating seqadj
(this one should require a new patch)
6. For CTA_HELP, updating helpinfo may be a problem(I am not sure
about this part)
7. For CTA_STATUS, I think it may cause a big problem, the bit set operation via
ctnetlink_change_status is not atomic, so it may clear the
IPS_DYING_BIT, for example:
CPU0(update CTA_STATUS)CPU1(packet path, set _DYING_)
ctnetlink_change_status --
olds = ct->status --
--   set_bit(IPS_DYING_BIT...
ct->status = olds | new status --> Here DYING_BIT will be cleared!

But I think we can convert "ct->status |= status & ~(IPS_NAT_DONE_MASK
| IPS_NAT_MASK);"
to a series of atomic bit set operations to solve the 7th issue.

And the issues listed above won't be solved by holding _expect_lock,
so I think we should get rid of the _expect_lock at first.
--
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 iptables] extensions: libxt_hashlimit: fix 64-bit printf formats

2017-04-10 Thread Jan Engelhardt

On Saturday 2017-04-08 22:29, Pablo Neira Ayuso wrote:
>> @@ -262,7 +262,7 @@ static uint64_t parse_burst(const char *burst, int 
>> revision)
>>  if (v > max)
>>  xtables_error(PARAMETER_PROBLEM, "bad value for option "
>>  "\"--hashlimit-burst\", value \"%s\" too large "
>> -"(max %lumb).", burst, max/1024/1024);
>> +"(max %" PRIu64 "mb).", burst, max/1024/1024);
>^  ^
>
>I can remove this whitespaces, right?

With my distro hat on:

Clumping these together like "foo"BAR"baz" has already caused compile failures
in the broader scope of distributions (thousands of packages) because languages
introduced new tokenization rules. Admittedly, this occurred in C++ (namely,
user-defined string literals), but it does show that tokens which logically are
separate should stay separate.
--
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


[GIT 0/3] Second Round of IPVS Updates for v4.12

2017-04-10 Thread Simon Horman
Hi Pablo,

please consider these clean-ups and enhancements to IPVS for v4.12.

* Removal unused variable
* Use kzalloc where appropriate
* More efficient detection of presence of NAT extension


The following changes since commit 592d42ac7fd36408979e09bf2f170f2595dab7b8:

  Merge branch 'qed-IOV-cleanups' (2017-03-21 19:02:38 -0700)

are available in the git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-next.git 
ipvs2-for-v4.12

for you to fetch changes up to e24113769960980579610ecfd657bb17f19373a3:

  ipvs: remove unused variable (2017-03-30 14:54:47 +0200)


Arushi Singhal (1):
  ipvs: remove unused variable

Florian Westphal (1):
  netfilter: ipvs: don't check for presence of nat extension

Varsha Rao (1):
  netfilter: ipvs: Replace kzalloc with kcalloc.

 net/netfilter/ipvs/ip_vs_ftp.c  | 7 ++-
 net/netfilter/ipvs/ip_vs_sync.c | 4 ++--
 2 files changed, 4 insertions(+), 7 deletions(-)
--
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/3] ipvs: remove unused variable

2017-04-10 Thread Simon Horman
From: Arushi Singhal 

This patch uses the following coccinelle script to remove
a variable that was simply used to store the return
value of a function call before returning it:

@@
identifier len,f;
@@

-int len;
 ... when != len
 when strict
-len =
+return
f(...);
-return len;

Signed-off-by: Arushi Singhal 
Signed-off-by: Simon Horman 
---
 net/netfilter/ipvs/ip_vs_ftp.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index 2e2bf7428cd1..6caf4459e981 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -482,11 +482,8 @@ static struct pernet_operations ip_vs_ftp_ops = {
 
 static int __init ip_vs_ftp_init(void)
 {
-   int rv;
-
-   rv = register_pernet_subsys(&ip_vs_ftp_ops);
/* rcu_barrier() is called by netns on error */
-   return rv;
+   return register_pernet_subsys(&ip_vs_ftp_ops);
 }
 
 /*
-- 
2.7.0.rc3.207.g0ac5344

--
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/3] netfilter: ipvs: Replace kzalloc with kcalloc.

2017-04-10 Thread Simon Horman
From: Varsha Rao 

Replace kzalloc with kcalloc. As kcalloc is preferred for allocating an
array instead of kzalloc. This patch fixes the checkpatch issue.

Signed-off-by: Varsha Rao 
---
 net/netfilter/ipvs/ip_vs_sync.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index b03c28084f81..30d6b2cc00a0 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -1849,7 +1849,7 @@ int start_sync_thread(struct netns_ipvs *ipvs, struct 
ipvs_sync_daemon_cfg *c,
if (state == IP_VS_STATE_MASTER) {
struct ipvs_master_sync_state *ms;
 
-   ipvs->ms = kzalloc(count * sizeof(ipvs->ms[0]), GFP_KERNEL);
+   ipvs->ms = kcalloc(count, sizeof(ipvs->ms[0]), GFP_KERNEL);
if (!ipvs->ms)
goto out;
ms = ipvs->ms;
@@ -1862,7 +1862,7 @@ int start_sync_thread(struct netns_ipvs *ipvs, struct 
ipvs_sync_daemon_cfg *c,
ms->ipvs = ipvs;
}
} else {
-   array = kzalloc(count * sizeof(struct task_struct *),
+   array = kcalloc(count, sizeof(struct task_struct *),
GFP_KERNEL);
if (!array)
goto out;
-- 
2.7.0.rc3.207.g0ac5344

--
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/3] netfilter: ipvs: don't check for presence of nat extension

2017-04-10 Thread Simon Horman
From: Florian Westphal 

Check for the NAT status bits, they are set once conntrack needs NAT in source 
or
reply direction, this is slightly faster than nfct_nat() as that has to check 
the
extension area.

Signed-off-by: Florian Westphal 
---
 net/netfilter/ipvs/ip_vs_ftp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index d30c327bb578..2e2bf7428cd1 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -260,7 +260,7 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct 
ip_vs_conn *cp,
buf_len = strlen(buf);
 
ct = nf_ct_get(skb, &ctinfo);
-   if (ct && !nf_ct_is_untracked(ct) && nfct_nat(ct)) {
+   if (ct && !nf_ct_is_untracked(ct) && (ct->status & 
IPS_NAT_MASK)) {
/* If mangling fails this function will return 0
 * which will cause the packet to be dropped.
 * Mangling can only fail under memory pressure,
-- 
2.7.0.rc3.207.g0ac5344

--
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/2 v2] iptables-restore/ip6tables-restore: add --version/-V argument

2017-04-10 Thread Dan Williams
Prints program version just like iptables/ip6tables.

Signed-off-by: Dan Williams 
---
v2: documented new options in manpage

 iptables/ip6tables-restore.c   | 15 +++
 iptables/iptables-restore.8.in |  7 +--
 iptables/iptables-restore.c| 10 --
 4 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/iptables/ip6tables-restore.c b/iptables/ip6tables-restore.c
index 30a4ade..419a2b0 100644
--- a/iptables/ip6tables-restore.c
+++ b/iptables/ip6tables-restore.c
@@ -36,6 +36,7 @@ static struct timeval wait_interval = {
 static const struct option options[] = {
{.name = "counters",  .has_arg = 0, .val = 'c'},
{.name = "verbose",   .has_arg = 0, .val = 'v'},
+   {.name = "version",   .has_arg = 0, .val = 'V'},
{.name = "test",  .has_arg = 0, .val = 't'},
{.name = "help",  .has_arg = 0, .val = 'h'},
{.name = "noflush",   .has_arg = 0, .val = 'n'},
@@ -48,11 +49,15 @@ static const struct option options[] = {
 
 static void print_usage(const char *name, const char *version) 
__attribute__((noreturn));
 
+#define prog_name ip6tables_globals.program_name
+#define prog_vers ip6tables_globals.program_version
+
 static void print_usage(const char *name, const char *version)
 {
-   fprintf(stderr, "Usage: %s [-c] [-v] [-t] [-h] [-n] [-w secs] [-W 
usecs] [-T table] [-M command]\n"
+   fprintf(stderr, "Usage: %s [-c] [-v] [-V] [-t] [-h] [-n] [-w secs] [-W 
usecs] [-T table] [-M command]\n"
"  [ --counters ]\n"
"  [ --verbose ]\n"
+   "  [ --version]\n"
"  [ --test ]\n"
"  [ --help ]\n"
"  [ --noflush ]\n"
@@ -78,8 +83,7 @@ static struct xtc_handle *create_handle(const char *tablename)
 
if (!handle) {
xtables_error(PARAMETER_PROBLEM, "%s: unable to initialize "
-   "table '%s'\n", ip6tables_globals.program_name,
-   tablename);
+   "table '%s'\n", prog_name, tablename);
exit(1);
}
return handle;
@@ -213,7 +217,7 @@ int ip6tables_restore_main(int argc, char *argv[])
init_extensions6();
 #endif
 
-   while ((c = getopt_long(argc, argv, "bcvthnwWM:T:", options, NULL)) != 
-1) {
+   while ((c = getopt_long(argc, argv, "bcvVthnwWM:T:", options, NULL)) != 
-1) {
switch (c) {
case 'b':
fprintf(stderr, "-b/--binary option is not 
implemented\n");
@@ -224,6 +228,9 @@ int ip6tables_restore_main(int argc, char *argv[])
case 'v':
verbose = 1;
break;
+   case 'V':
+   printf("%s v%s\n", prog_name, prog_vers);
+   exit(0);
case 't':
testing = 1;
break;
diff --git a/iptables/iptables-restore.8.in b/iptables/iptables-restore.8.in
index 7a286b9..bba505d 100644
--- a/iptables/iptables-restore.8.in
+++ b/iptables/iptables-restore.8.in
@@ -23,10 +23,10 @@ iptables-restore \(em Restore IP Tables
 .P
 ip6tables-restore \(em Restore IPv6 Tables
 .SH SYNOPSIS
-\fBiptables\-restore\fP [\fB\-chntv\fP] [\fB\-M\fP \fImodprobe\fP]
+\fBiptables\-restore\fP [\fB\-chntvV\fP] [\fB\-M\fP \fImodprobe\fP]
 [\fB\-T\fP \fIname\fP] [\fBfile\fP]
 .P
-\fBip6tables\-restore\fP [\fB\-chntv\fP] [\fB\-M\fP \fImodprobe\fP]
+\fBip6tables\-restore\fP [\fB\-chntvV\fP] [\fB\-M\fP \fImodprobe\fP]
 [\fB\-T\fP \fIname\fP] [\fBfile\fP]
 .SH DESCRIPTION
 .PP
@@ -53,6 +53,9 @@ Only parse and construct the ruleset, but do not commit it.
 \fB\-v\fP, \fB\-\-verbose\fP
 Print additional debug info during ruleset processing.
 .TP
+\fB\-V\fP, \fB\-\-version\fP
+Print the program version number.
+.TP
 \fB\-M\fP, \fB\-\-modprobe\fP \fImodprobe_program\fP
 Specify the path to the modprobe program. By default, iptables-restore will
 inspect /proc/sys/kernel/modprobe to determine the executable's path.
diff --git a/iptables/iptables-restore.c b/iptables/iptables-restore.c
index 15db358..cb06559 100644
--- a/iptables/iptables-restore.c
+++ b/iptables/iptables-restore.c
@@ -33,6 +33,7 @@ static struct timeval wait_interval = {
 static const struct option options[] = {
{.name = "counters",  .has_arg = 0, .val = 'c'},
{.name = "verbose",   .has_arg = 0, .val = 'v'},
+   {.name = "version",   .has_arg = 0, .val = 'V'},
{.name = "test",  .has_arg = 0, .val = 't'},
{.name = "help",  .has_arg = 0, .val = 'h'},
{.name = "noflush",   .has_arg = 0, .val = 'n'},
@@ -46,12 +47,14 @@ static const struct option options[] = {
 static void print_usage(const char *name, const char *

[PATCH 2/2 v2] iptables-restore.8: document -w/-W options

2017-04-10 Thread Dan Williams
Fixes: 999eaa241212d3952ddff39a99d0d55a74e3639e

Signed-off-by: Dan Williams 
---
 iptables/iptables-restore.8.in | 25 +
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/iptables/iptables-restore.8.in b/iptables/iptables-restore.8.in
index bba505d..f751492 100644
--- a/iptables/iptables-restore.8.in
+++ b/iptables/iptables-restore.8.in
@@ -23,11 +23,13 @@ iptables-restore \(em Restore IP Tables
 .P
 ip6tables-restore \(em Restore IPv6 Tables
 .SH SYNOPSIS
-\fBiptables\-restore\fP [\fB\-chntvV\fP] [\fB\-M\fP \fImodprobe\fP]
-[\fB\-T\fP \fIname\fP] [\fBfile\fP]
+\fBiptables\-restore\fP [\fB\-chntvV\fP] [\fB\-w\fP \fIsecs\fP]
+[\fB\-W\fP \fIusecs\fP] [\fB\-M\fP \fImodprobe\fP] [\fB\-T\fP \fIname\fP]
+[\fBfile\fP]
 .P
-\fBip6tables\-restore\fP [\fB\-chntvV\fP] [\fB\-M\fP \fImodprobe\fP]
-[\fB\-T\fP \fIname\fP] [\fBfile\fP]
+\fBip6tables\-restore\fP [\fB\-chntvV\fP] [\fB\-w\fP \fIsecs\fP]
+[\fB\-W\fP \fIusecs\fP] [\fB\-M\fP \fImodprobe\fP] [\fB\-T\fP \fIname\fP]
+[\fBfile\fP]
 .SH DESCRIPTION
 .PP
 .B iptables-restore
@@ -56,6 +58,21 @@ Print additional debug info during ruleset processing.
 \fB\-V\fP, \fB\-\-version\fP
 Print the program version number.
 .TP
+\fB\-w\fP, \fB\-\-wait\fP [\fIseconds\fP]
+Wait for the xtables lock.
+To prevent multiple instances of the program from running concurrently,
+an attempt will be made to obtain an exclusive lock at launch.  By default,
+the program will exit if the lock cannot be obtained.  This option will
+make the program wait (indefinitely or for optional \fIseconds\fP) until
+the exclusive lock can be obtained.
+.TP
+\fB\-W\fP, \fB\-\-wait-interval\fP \fImicroseconds\fP
+Interval to wait per each iteration.
+When running latency sensitive applications, waiting for the xtables lock
+for extended durations may not be acceptable. This option will make each
+iteration take the amount of time specified. The default interval is
+1 second. This option only works with \fB\-w\fP.
+.TP
 \fB\-M\fP, \fB\-\-modprobe\fP \fImodprobe_program\fP
 Specify the path to the modprobe program. By default, iptables-restore will
 inspect /proc/sys/kernel/modprobe to determine the executable's path.
-- 
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] ipvs: remove unused function ip_vs_set_state_timeout

2017-04-10 Thread Aaron Conole
There are no in-tree callers of this function and it isn't exported.

Signed-off-by: Aaron Conole 
---
 include/net/ip_vs.h  |  2 --
 net/netfilter/ipvs/ip_vs_proto.c | 22 --
 2 files changed, 24 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 8a4a57b8..c76fedb 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -1349,8 +1349,6 @@ int ip_vs_protocol_init(void);
 void ip_vs_protocol_cleanup(void);
 void ip_vs_protocol_timeout_change(struct netns_ipvs *ipvs, int flags);
 int *ip_vs_create_timeout_table(int *table, int size);
-int ip_vs_set_state_timeout(int *table, int num, const char *const *names,
-   const char *name, int to);
 void ip_vs_tcpudp_debug_packet(int af, struct ip_vs_protocol *pp,
   const struct sk_buff *skb, int offset,
   const char *msg);
diff --git a/net/netfilter/ipvs/ip_vs_proto.c b/net/netfilter/ipvs/ip_vs_proto.c
index 8ae4807..ca880a3 100644
--- a/net/netfilter/ipvs/ip_vs_proto.c
+++ b/net/netfilter/ipvs/ip_vs_proto.c
@@ -193,28 +193,6 @@ ip_vs_create_timeout_table(int *table, int size)
 }
 
 
-/*
- * Set timeout value for state specified by name
- */
-int
-ip_vs_set_state_timeout(int *table, int num, const char *const *names,
-   const char *name, int to)
-{
-   int i;
-
-   if (!table || !name || !to)
-   return -EINVAL;
-
-   for (i = 0; i < num; i++) {
-   if (strcmp(names[i], name))
-   continue;
-   table[i] = to * HZ;
-   return 0;
-   }
-   return -ENOENT;
-}
-
-
 const char * ip_vs_state_name(__u16 proto, int state)
 {
struct ip_vs_protocol *pp = ip_vs_proto_get(proto);
-- 
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] ipset: remove unused function __ip_set_get_netlink

2017-04-10 Thread Aaron Conole
There are no in-tree callers.

Signed-off-by: Aaron Conole 
---
 net/netfilter/ipset/ip_set_core.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_core.c 
b/net/netfilter/ipset/ip_set_core.c
index c296f9b..68ba531 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -501,14 +501,6 @@ __ip_set_put(struct ip_set *set)
  * a separate reference counter
  */
 static inline void
-__ip_set_get_netlink(struct ip_set *set)
-{
-   write_lock_bh(&ip_set_ref_lock);
-   set->ref_netlink++;
-   write_unlock_bh(&ip_set_ref_lock);
-}
-
-static inline void
 __ip_set_put_netlink(struct ip_set *set)
 {
write_lock_bh(&ip_set_ref_lock);
-- 
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


[libnetfilter_queue][PATCH 2/2] Declare the define visivility attribute together

2017-04-10 Thread Khem Raj
clang ignores the visibility attribute if its not
defined before the definition. As a result these
symbols become hidden and consumers of this library
fail to link due to these missing symbols

Signed-off-by: Khem Raj 
---
 doxygen.cfg.in   |   2 +-
 src/extra/ipv4.c |  15 +++
 src/extra/ipv6.c |   9 ++--
 src/extra/pktbuff.c  |  42 ++
 src/extra/tcp.c  |  21 +++--
 src/extra/udp.c  |  21 +++--
 src/internal.h   |   5 +--
 src/libnetfilter_queue.c | 108 ---
 src/nlmsg.c  |  21 +++--
 9 files changed, 82 insertions(+), 162 deletions(-)

diff --git a/doxygen.cfg.in b/doxygen.cfg.in
index a7378ca..659abee 100644
--- a/doxygen.cfg.in
+++ b/doxygen.cfg.in
@@ -72,7 +72,7 @@ RECURSIVE  = YES
 EXCLUDE= 
 EXCLUDE_SYMLINKS   = NO
 EXCLUDE_PATTERNS   = 
-EXCLUDE_SYMBOLS= EXPORT_SYMBOL
+EXCLUDE_SYMBOLS=
 EXAMPLE_PATH   = 
 EXAMPLE_PATTERNS   = 
 EXAMPLE_RECURSIVE  = NO
diff --git a/src/extra/ipv4.c b/src/extra/ipv4.c
index a93d113..56d5dc7 100644
--- a/src/extra/ipv4.c
+++ b/src/extra/ipv4.c
@@ -32,7 +32,7 @@
  * This funcion returns NULL if the IPv4 is malformed or the protocol version
  * is not 4. On success, it returns a valid pointer to the IPv4 header.
  */
-struct iphdr *nfq_ip_get_hdr(struct pkt_buff *pktb)
+struct iphdr __EXPORTED *nfq_ip_get_hdr(struct pkt_buff *pktb)
 {
struct iphdr *iph;
unsigned int pktlen = pktb->tail - pktb->network_header;
@@ -53,14 +53,13 @@ struct iphdr *nfq_ip_get_hdr(struct pkt_buff *pktb)
 
return iph;
 }
-EXPORT_SYMBOL(nfq_ip_get_hdr);
 
 /**
  * nfq_ip_set_transport_header - set transport header
  * \param pktb: pointer to network packet buffer
  * \param iph: pointer to the IPv4 header
  */
-int nfq_ip_set_transport_header(struct pkt_buff *pktb, struct iphdr *iph)
+int __EXPORTED nfq_ip_set_transport_header(struct pkt_buff *pktb, struct iphdr 
*iph)
 {
int doff = iph->ihl * 4;
 
@@ -71,7 +70,6 @@ int nfq_ip_set_transport_header(struct pkt_buff *pktb, struct 
iphdr *iph)
pktb->transport_header = pktb->network_header + doff;
return 0;
 }
-EXPORT_SYMBOL(nfq_ip_set_transport_header);
 
 /**
  * nfq_ip_set_checksum - set IPv4 checksum
@@ -80,14 +78,13 @@ EXPORT_SYMBOL(nfq_ip_set_transport_header);
  * \note Call to this function if you modified the IPv4 header to update the
  * checksum.
  */
-void nfq_ip_set_checksum(struct iphdr *iph)
+void __EXPORTED nfq_ip_set_checksum(struct iphdr *iph)
 {
uint32_t iph_len = iph->ihl * 4;
 
iph->check = 0;
iph->check = nfq_checksum(0, (uint16_t *)iph, iph_len);
 }
-EXPORT_SYMBOL(nfq_ip_set_checksum);
 
 /**
  * nfq_ip_mangle - mangle IPv4 packet buffer
@@ -100,7 +97,7 @@ EXPORT_SYMBOL(nfq_ip_set_checksum);
  *
  * \note This function recalculates the IPv4 checksum (if needed).
  */
-int nfq_ip_mangle(struct pkt_buff *pkt, unsigned int dataoff,
+int __EXPORTED nfq_ip_mangle(struct pkt_buff *pkt, unsigned int dataoff,
  unsigned int match_offset, unsigned int match_len,
  const char *rep_buffer, unsigned int rep_len)
 {
@@ -116,7 +113,6 @@ int nfq_ip_mangle(struct pkt_buff *pkt, unsigned int 
dataoff,
 
return 1;
 }
-EXPORT_SYMBOL(nfq_ip_mangle);
 
 /**
  * nfq_pkt_snprintf_ip - print IPv4 header into buffer in iptables LOG format
@@ -128,7 +124,7 @@ EXPORT_SYMBOL(nfq_ip_mangle);
  * case that there is enough room in the buffer. Read snprintf manpage for more
  * information to know more about this strange behaviour.
  */
-int nfq_ip_snprintf(char *buf, size_t size, const struct iphdr *iph)
+int __EXPORTED nfq_ip_snprintf(char *buf, size_t size, const struct iphdr *iph)
 {
int ret;
struct in_addr src = { iph->saddr };
@@ -147,7 +143,6 @@ int nfq_ip_snprintf(char *buf, size_t size, const struct 
iphdr *iph)
 
return ret;
 }
-EXPORT_SYMBOL(nfq_ip_snprintf);
 
 /**
  * @}
diff --git a/src/extra/ipv6.c b/src/extra/ipv6.c
index 7c5dc9b..6641c6b 100644
--- a/src/extra/ipv6.c
+++ b/src/extra/ipv6.c
@@ -33,7 +33,7 @@
  * This funcion returns NULL if an invalid header is found. On sucess, it
  * returns a valid pointer to the header.
  */
-struct ip6_hdr *nfq_ip6_get_hdr(struct pkt_buff *pktb)
+struct ip6_hdr __EXPORTED *nfq_ip6_get_hdr(struct pkt_buff *pktb)
 {
struct ip6_hdr *ip6h;
unsigned int pktlen = pktb->tail - pktb->network_header;
@@ -50,7 +50,6 @@ struct ip6_hdr *nfq_ip6_get_hdr(struct pkt_buff *pktb)
 
return ip6h;
 }
-EXPORT_SYMBOL(nfq_ip6_get_hdr);
 
 /**
  * nfq_ip6_set_transport_header - set transport header pointer for IPv6 packet
@@ -61,7 +60,7 @@ EXPORT_SYMBOL(nfq_ip6_get_hdr);
  * This function returns 1 if the protocol has been found and the transport
  * header has been set. Otherwise, it returns 0.
  */
-int nfq_ip6_set_transport_header(struct pkt_buff *pktb, struct ip6_hd

[libnetfilter_queue][PATCH 1/2] Correct typo in the location of internal.h in #include

2017-04-10 Thread Khem Raj
Signed-off-by: Khem Raj 

%% original patch: 
0001-Correct-typo-in-the-location-of-internal.h-in-includ.patch
---
 src/libnetfilter_queue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libnetfilter_queue.c b/src/libnetfilter_queue.c
index 211a8ba..065d618 100644
--- a/src/libnetfilter_queue.c
+++ b/src/libnetfilter_queue.c
@@ -32,7 +32,7 @@
 
 #include 
 #include 
-#include "src/internal.h"
+#include "internal.h"
 
 /**
  * \mainpage
-- 
2.12.2

--
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


[libnftnl][PATCH] Declare the define visivility attribute together

2017-04-10 Thread Khem Raj
clang ignores the visibility attribute if its not
defined before the definition. As a result these
symbols become hidden and consumers of this library
fail to link due to these missing symbols

Signed-off-by: Khem Raj 
---
 doxygen.cfg.in  |   2 +-
 include/utils.h |   5 +--
 src/batch.c |  21 ---
 src/chain.c | 102 +-
 src/common.c|  21 ---
 src/expr.c  |  51 +
 src/gen.c   |  39 +++
 src/object.c|  99 
 src/rule.c  | 114 +++-
 src/ruleset.c   |  48 
 src/set.c   |  96 ---
 src/set_elem.c  |  72 ---
 src/table.c |  90 +++-
 src/trace.c |  27 +-
 src/udata.c |  48 
 15 files changed, 279 insertions(+), 556 deletions(-)

diff --git a/doxygen.cfg.in b/doxygen.cfg.in
index 23fcad4..e49f28d 100644
--- a/doxygen.cfg.in
+++ b/doxygen.cfg.in
@@ -72,7 +72,7 @@ RECURSIVE  = YES
 EXCLUDE= 
 EXCLUDE_SYMLINKS   = NO
 EXCLUDE_PATTERNS   = */.git/* .*.d
-EXCLUDE_SYMBOLS= EXPORT_SYMBOL
+EXCLUDE_SYMBOLS=
 EXAMPLE_PATH   = 
 EXAMPLE_PATTERNS   = 
 EXAMPLE_RECURSIVE  = NO
diff --git a/include/utils.h b/include/utils.h
index 2f5cf34..ff8207e 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -9,10 +9,9 @@
 
 #include "config.h"
 #ifdef HAVE_VISIBILITY_HIDDEN
-#  define __visible__attribute__((visibility("default")))
-#  define EXPORT_SYMBOL(x) typeof(x) (x) __visible;
+#  define __EXPORTED   __attribute__((visibility("default")))
 #else
-#  define EXPORT_SYMBOL
+#  define __EXPORT
 #endif
 
 #define __noreturn __attribute__((__noreturn__))
diff --git a/src/batch.c b/src/batch.c
index 5ee3fd7..3bedd26 100644
--- a/src/batch.c
+++ b/src/batch.c
@@ -57,7 +57,7 @@ static void nftnl_batch_add_page(struct nftnl_batch_page 
*page,
list_add_tail(&page->head, &batch->page_list);
 }
 
-struct nftnl_batch *nftnl_batch_alloc(uint32_t pg_size, uint32_t 
pg_overrun_size)
+struct nftnl_batch __EXPORTED *nftnl_batch_alloc(uint32_t pg_size, uint32_t 
pg_overrun_size)
 {
struct nftnl_batch *batch;
struct nftnl_batch_page *page;
@@ -80,9 +80,8 @@ err1:
free(batch);
return NULL;
 }
-EXPORT_SYMBOL(nftnl_batch_alloc);
 
-void nftnl_batch_free(struct nftnl_batch *batch)
+void __EXPORTED nftnl_batch_free(struct nftnl_batch *batch)
 {
struct nftnl_batch_page *page, *next;
 
@@ -94,9 +93,8 @@ void nftnl_batch_free(struct nftnl_batch *batch)
 
free(batch);
 }
-EXPORT_SYMBOL(nftnl_batch_free);
 
-int nftnl_batch_update(struct nftnl_batch *batch)
+int __EXPORTED nftnl_batch_update(struct nftnl_batch *batch)
 {
struct nftnl_batch_page *page;
struct nlmsghdr *last_nlh;
@@ -119,21 +117,18 @@ int nftnl_batch_update(struct nftnl_batch *batch)
 err1:
return -1;
 }
-EXPORT_SYMBOL(nftnl_batch_update);
 
-void *nftnl_batch_buffer(struct nftnl_batch *batch)
+void __EXPORTED *nftnl_batch_buffer(struct nftnl_batch *batch)
 {
return mnl_nlmsg_batch_current(batch->current_page->batch);
 }
-EXPORT_SYMBOL(nftnl_batch_buffer);
 
-uint32_t nftnl_batch_buffer_len(struct nftnl_batch *batch)
+uint32_t __EXPORTED nftnl_batch_buffer_len(struct nftnl_batch *batch)
 {
return mnl_nlmsg_batch_size(batch->current_page->batch);
 }
-EXPORT_SYMBOL(nftnl_batch_buffer_len);
 
-int nftnl_batch_iovec_len(struct nftnl_batch *batch)
+int __EXPORTED nftnl_batch_iovec_len(struct nftnl_batch *batch)
 {
int num_pages = batch->num_pages;
 
@@ -143,9 +138,8 @@ int nftnl_batch_iovec_len(struct nftnl_batch *batch)
 
return num_pages;
 }
-EXPORT_SYMBOL(nftnl_batch_iovec_len);
 
-void nftnl_batch_iovec(struct nftnl_batch *batch, struct iovec *iov,
+void __EXPORTED nftnl_batch_iovec(struct nftnl_batch *batch, struct iovec *iov,
   uint32_t iovlen)
 {
struct nftnl_batch_page *page;
@@ -160,4 +154,3 @@ void nftnl_batch_iovec(struct nftnl_batch *batch, struct 
iovec *iov,
i++;
}
 }
-EXPORT_SYMBOL(nftnl_batch_iovec);
diff --git a/src/chain.c b/src/chain.c
index 29860c5..362fa0d 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -87,13 +87,12 @@ static const char *nftnl_hooknum2str(int family, int 
hooknum)
return "unknown";
 }
 
-struct nftnl_chain *nftnl_chain_alloc(void)
+struct nftnl_chain __EXPORTED *nftnl_chain_alloc(void)
 {
return calloc(1, sizeof(struct nftnl_chain));
 }
-EXPORT_SYMBOL(nftnl_chain_alloc);
 
-void nftnl_chain_free(const struct nftnl_chain *c)
+void __EXPORTED nftnl_chain_free(const struct nftnl_chain *c)
 {
if (c->flags & (1 << NFTNL_CHAIN_NAME))
xfree(c->name);
@@

[libnetfilter-acct][PATCH] Declare the define visibility attribute together

2017-04-10 Thread Khem Raj
clang ignores the visibility attribute if its not
defined before the definition. As a result these
symbols become hidden and consumers of this library
fail to link due to these missing symbols

Signed-off-by: Khem Raj 
---
 doxygen.cfg.in  |  2 +-
 src/internal.h  |  5 ++---
 src/libnetfilter_acct.c | 41 ++---
 3 files changed, 17 insertions(+), 31 deletions(-)

diff --git a/doxygen.cfg.in b/doxygen.cfg.in
index 7f4bd04..fe64d48 100644
--- a/doxygen.cfg.in
+++ b/doxygen.cfg.in
@@ -72,7 +72,7 @@ RECURSIVE  = YES
 EXCLUDE= 
 EXCLUDE_SYMLINKS   = NO
 EXCLUDE_PATTERNS   = */.git/* .*.d
-EXCLUDE_SYMBOLS= EXPORT_SYMBOL nfacct
+EXCLUDE_SYMBOLS= nfacct
 EXAMPLE_PATH   = 
 EXAMPLE_PATTERNS   = 
 EXAMPLE_RECURSIVE  = NO
diff --git a/src/internal.h b/src/internal.h
index f0cc2e1..e5c5ffd 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -3,10 +3,9 @@
 
 #include "config.h"
 #ifdef HAVE_VISIBILITY_HIDDEN
-#  define __visible__attribute__((visibility("default")))
-#  define EXPORT_SYMBOL(x) typeof(x) (x) __visible
+#  define __EXPORT __attribute__((visibility("default")))
 #else
-#  define EXPORT_SYMBOL
+#  define __EXPORT
 #endif
 
 #include 
diff --git a/src/libnetfilter_acct.c b/src/libnetfilter_acct.c
index b0bcf67..0220d14 100644
--- a/src/libnetfilter_acct.c
+++ b/src/libnetfilter_acct.c
@@ -76,21 +76,19 @@ struct nfacct {
  * In case of success, this function returns a valid pointer, otherwise NULL
  * s returned and errno is appropriately set.
  */
-struct nfacct *nfacct_alloc(void)
+struct nfacct __EXPORT *nfacct_alloc(void)
 {
return calloc(1, sizeof(struct nfacct));
 }
-EXPORT_SYMBOL(nfacct_alloc);
 
 /**
  * nfacct_free - release one accounting object
  * \param nfacct pointer to the accounting object
  */
-void nfacct_free(struct nfacct *nfacct)
+void __EXPORT nfacct_free(struct nfacct *nfacct)
 {
free(nfacct);
 }
-EXPORT_SYMBOL(nfacct_free);
 
 /**
  * nfacct_attr_set - set one attribute of the accounting object
@@ -98,7 +96,7 @@ EXPORT_SYMBOL(nfacct_free);
  * \param type attribute type you want to set
  * \param data pointer to data that will be used to set this attribute
  */
-void
+void __EXPORT
 nfacct_attr_set(struct nfacct *nfacct, enum nfacct_attr_type type,
const void *data)
 {
@@ -126,7 +124,6 @@ nfacct_attr_set(struct nfacct *nfacct, enum 
nfacct_attr_type type,
break;
}
 }
-EXPORT_SYMBOL(nfacct_attr_set);
 
 /**
  * nfacct_attr_set_str - set one attribute the accounting object
@@ -134,13 +131,12 @@ EXPORT_SYMBOL(nfacct_attr_set);
  * \param type attribute type you want to set
  * \param name string that will be used to set this attribute
  */
-void
+void __EXPORT
 nfacct_attr_set_str(struct nfacct *nfacct, enum nfacct_attr_type type,
const char *name)
 {
nfacct_attr_set(nfacct, type, name);
 }
-EXPORT_SYMBOL(nfacct_attr_set_str);
 
 /**
  * nfacct_attr_set_u64 - set one attribute the accounting object
@@ -148,20 +144,19 @@ EXPORT_SYMBOL(nfacct_attr_set_str);
  * \param type attribute type you want to set
  * \param value unsigned 64-bits integer
  */
-void
+void __EXPORT
 nfacct_attr_set_u64(struct nfacct *nfacct, enum nfacct_attr_type type,
uint64_t value)
 {
nfacct_attr_set(nfacct, type, &value);
 }
-EXPORT_SYMBOL(nfacct_attr_set_u64);
 
 /**
  * nfacct_attr_unset - unset one attribute the accounting object
  * \param nfacct pointer to the accounting object
  * \param type attribute type you want to set
  */
-void
+void __EXPORT
 nfacct_attr_unset(struct nfacct *nfacct, enum nfacct_attr_type type)
 {
switch(type) {
@@ -182,7 +177,6 @@ nfacct_attr_unset(struct nfacct *nfacct, enum 
nfacct_attr_type type)
break;
}
 }
-EXPORT_SYMBOL(nfacct_attr_unset);
 
 /**
  * nfacct_attr_get - get one attribute the accounting object
@@ -192,7 +186,7 @@ EXPORT_SYMBOL(nfacct_attr_unset);
  * This function returns a valid pointer to the attribute data. If a
  * unsupported attribute is used, this returns NULL.
  */
-const void *nfacct_attr_get(struct nfacct *nfacct, enum nfacct_attr_type type)
+const void __EXPORT *nfacct_attr_get(struct nfacct *nfacct, enum 
nfacct_attr_type type)
 {
const void *ret = NULL;
 
@@ -220,7 +214,6 @@ const void *nfacct_attr_get(struct nfacct *nfacct, enum 
nfacct_attr_type type)
}
return ret;
 }
-EXPORT_SYMBOL(nfacct_attr_get);
 
 /**
  * nfacct_attr_get_str - get one attribute the accounting object
@@ -230,12 +223,11 @@ EXPORT_SYMBOL(nfacct_attr_get);
  * This function returns a valid pointer to the beginning of the string.
  * If the attribute is unsupported, this returns NULL.
  */
-const char *
+const char __EXPORT *
 nfacct_attr_get_str(struct nfacct *nfacct, enum nfacct_attr_type type)
 {
return nfacct_attr_get(nfacct, type);
 }
-EXPORT_SYMBOL(nfacct_attr

RE: [PATCH nf 1/1] netfilter: seqadj: Fix possible non-linear data access for TCP header

2017-04-10 Thread 高峰
Hi Pablo,

> -Original Message-
> From: Pablo Neira Ayuso [mailto:pa...@netfilter.org]
> Sent: Monday, April 10, 2017 8:07 PM
> To: gfree.w...@foxmail.com
> Cc: netfilter-devel@vger.kernel.org; Gao Feng 
> Subject: Re: [PATCH nf 1/1] netfilter: seqadj: Fix possible non-linear
data access
> for TCP header
> 
> On Mon, Apr 10, 2017 at 06:36:03PM +0800, gfree.w...@foxmail.com wrote:
> > From: Gao Feng 
> >
> > The current call path of nf_ct_tcp_seqadj_set is the following.
> >
> > nfqnl_recv_verdict->ctnetlink_glue_hook->ctnetlink_glue_seqadj
> > ->nf_ct_tcp_seqadj_set.
> >
> > It couldn't make sure the TCP header is in the linear data part.
> > So use the skb_header_pointer instead of the current codes.
> >
> > BTW, the nf_ct_tcp_seqadj_set is one external function of netfilter
> > which works in the network layer, it should not assume the transport
> > header is in the linear data.
> >
> > Signed-off-by: Gao Feng 
> > ---
> >  net/netfilter/nf_conntrack_seqadj.c | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/netfilter/nf_conntrack_seqadj.c
> > b/net/netfilter/nf_conntrack_seqadj.c
> > index ef7063e..80394ab 100644
> > --- a/net/netfilter/nf_conntrack_seqadj.c
> > +++ b/net/netfilter/nf_conntrack_seqadj.c
> > @@ -61,11 +61,14 @@ void nf_ct_tcp_seqadj_set(struct sk_buff *skb,
> >   s32 off)
> >  {
> > const struct tcphdr *th;
> > +   struct tcphdr tcph;
> >
> > if (nf_ct_protonum(ct) != IPPROTO_TCP)
> > return;
> >
> > -   th = (struct tcphdr *)(skb_network_header(skb) + ip_hdrlen(skb));
> > +   th = skb_header_pointer(skb, ip_hdrlen(skb), sizeof(tcph), &tcph);
> > +   if (!th)
> > +   return;
> 
> This fix is required since your recent upgrade to check for 4 bytes
instead of 8
> bytes from conntrack. Please confirm this.

Do you mean the commit e5e693ab49a95e1994979972eea224eefa81eba9 "netfilter:
conntrack: Only need first 4 bytes to get l4proto ports"?
Sorry, I could not figure out why it would break the rule, and cause issue.

Let's look at one change for TCP of that commit as following

diff --git a/net/netfilter/nf_conntrack_proto_tcp.c
b/net/netfilter/nf_conntrack_proto_tcp.c
index 70c8381..4abe9e1 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -282,8 +282,8 @@ static bool tcp_pkt_to_tuple(const struct sk_buff *skb,
unsigned int dataoff,
const struct tcphdr *hp;
struct tcphdr _hdr;

-   /* Actually only need first 8 bytes. */
-   hp = skb_header_pointer(skb, dataoff, 8, &_hdr);
+   /* Actually only need first 4 bytes to get ports. */
+   hp = skb_header_pointer(skb, dataoff, 4, &_hdr);
if (hp == NULL)
return false;

The skb_header_pointer only copies the data, not make the data linear.
So I think it is ok after 4 bytes instead of 8 bytes.
It only affect the local variable, not the skb.

> 
> If so, it would be good to review the NAT code to see if there are more
spots
> that are not broken since that change...

I also check the other codes of Netfilter.

grep -in skb_network_header * in nf/net/netfilter and
nf/net/ipv4(6)/netfilter/
nf_nat_helper.c:41: data = skb_network_header(skb) + dataoff;
nf_tables_core.c:99:ptr = skb_network_header(skb) +
pkt->xt.thoff;
xt_TCPMSS.c:104:tcph = (struct tcphdr *)(skb_network_header(skb) +
tcphoff);
xt_TCPMSS.c:163:tcph = (struct tcphdr
*)(skb_network_header(skb) + tcphoff);
xt_TCPOPTSTRIP.c:54:tcph = (struct tcphdr *)(skb_network_header(skb) +
tcphoff);
arpt_mangle.c:23:   arpptr = skb_network_header(skb) + sizeof(*arp);

all of them make sure the skb is linear, except the nf_tables_core.c which
use the skb_tail_pointer to exclude the non-linear data.

So there is only nf_ct_tcp_seqadj_set which need fixed.

Best 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 1/1] netfilter: seqadj: Fix possible non-linear data access for TCP header

2017-04-10 Thread Gao Feng
Hi Pablo,

> -Original Message-
> From: Pablo Neira Ayuso [mailto:pa...@netfilter.org]
> Sent: Monday, April 10, 2017 8:07 PM
> To: gfree.w...@foxmail.com
> Cc: netfilter-devel@vger.kernel.org; Gao Feng 
> Subject: Re: [PATCH nf 1/1] netfilter: seqadj: Fix possible non-linear
data access
> for TCP header
> 
> On Mon, Apr 10, 2017 at 06:36:03PM +0800, gfree.w...@foxmail.com wrote:
> > From: Gao Feng 
> >
> > The current call path of nf_ct_tcp_seqadj_set is the following.
> >
> > nfqnl_recv_verdict->ctnetlink_glue_hook->ctnetlink_glue_seqadj
> > ->nf_ct_tcp_seqadj_set.
> >
> > It couldn't make sure the TCP header is in the linear data part.
> > So use the skb_header_pointer instead of the current codes.
> >
> > BTW, the nf_ct_tcp_seqadj_set is one external function of netfilter
> > which works in the network layer, it should not assume the transport
> > header is in the linear data.
> >
> > Signed-off-by: Gao Feng 
> > ---
> >  net/netfilter/nf_conntrack_seqadj.c | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/netfilter/nf_conntrack_seqadj.c
> > b/net/netfilter/nf_conntrack_seqadj.c
> > index ef7063e..80394ab 100644
> > --- a/net/netfilter/nf_conntrack_seqadj.c
> > +++ b/net/netfilter/nf_conntrack_seqadj.c
> > @@ -61,11 +61,14 @@ void nf_ct_tcp_seqadj_set(struct sk_buff *skb,
> >   s32 off)
> >  {
> > const struct tcphdr *th;
> > +   struct tcphdr tcph;
> >
> > if (nf_ct_protonum(ct) != IPPROTO_TCP)
> > return;
> >
> > -   th = (struct tcphdr *)(skb_network_header(skb) + ip_hdrlen(skb));
> > +   th = skb_header_pointer(skb, ip_hdrlen(skb), sizeof(tcph), &tcph);
> > +   if (!th)
> > +   return;
> 
> This fix is required since your recent upgrade to check for 4 bytes
instead of 8
> bytes from conntrack. Please confirm this.
Do you mean the commit e5e693ab49a95e1994979972eea224eefa81eba9 "netfilter:
conntrack: Only need first 4 bytes to get l4proto ports"?
Sorry, I could not figure out why it would break the rule, and cause issue.

Let's look at one change for TCP of that commit as following

diff --git a/net/netfilter/nf_conntrack_proto_tcp.c
b/net/netfilter/nf_conntrack_proto_tcp.c
index 70c8381..4abe9e1 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -282,8 +282,8 @@ static bool tcp_pkt_to_tuple(const struct sk_buff *skb,
unsigned int dataoff,
const struct tcphdr *hp;
struct tcphdr _hdr;

-   /* Actually only need first 8 bytes. */
-   hp = skb_header_pointer(skb, dataoff, 8, &_hdr);
+   /* Actually only need first 4 bytes to get ports. */
+   hp = skb_header_pointer(skb, dataoff, 4, &_hdr);
if (hp == NULL)
return false;

The skb_header_pointer only copies the data, not make the data linear.
So I think it is ok after 4 bytes instead of 8 bytes.
It only affect the local variable, not the skb.

> 
> If so, it would be good to review the NAT code to see if there are more
spots
> that are not broken since that change...

I also check the other codes of Netfilter.

grep -in skb_network_header * in nf/net/netfilter and
nf/net/ipv4(6)/netfilter/
nf_nat_helper.c:41: data = skb_network_header(skb) + dataoff;
nf_tables_core.c:99:ptr = skb_network_header(skb) +
pkt->xt.thoff;
xt_TCPMSS.c:104:tcph = (struct tcphdr *)(skb_network_header(skb) +
tcphoff);
xt_TCPMSS.c:163:tcph = (struct tcphdr
*)(skb_network_header(skb) + tcphoff);
xt_TCPOPTSTRIP.c:54:tcph = (struct tcphdr *)(skb_network_header(skb) +
tcphoff);
arpt_mangle.c:23:   arpptr = skb_network_header(skb) + sizeof(*arp);

all of them make sure the skb is linear, except the nf_tables_core.c which
use the skb_tail_pointer to exclude the non-linear data.

So there is only nf_ct_tcp_seqadj_set which need fixed.

Best 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 1/1] netfilter: seqadj: Fix possible non-linear data access for TCP header

2017-04-10 Thread 高峰

> -Original Message-
> From: 高峰 [mailto:f...@ikuai8.com]
> Sent: Tuesday, April 11, 2017 9:00 AM
> To: 'Pablo Neira Ayuso' ; 'gfree.w...@foxmail.com'
> 
> Cc: 'netfilter-devel@vger.kernel.org' 
> Subject: RE: [PATCH nf 1/1] netfilter: seqadj: Fix possible non-linear
data access
> for TCP header
> 
> Hi Pablo,
> 
> > -Original Message-
> > From: Pablo Neira Ayuso [mailto:pa...@netfilter.org]
> > Sent: Monday, April 10, 2017 8:07 PM
> > To: gfree.w...@foxmail.com
> > Cc: netfilter-devel@vger.kernel.org; Gao Feng 
> > Subject: Re: [PATCH nf 1/1] netfilter: seqadj: Fix possible non-linear
> > data access for TCP header
> >
> > On Mon, Apr 10, 2017 at 06:36:03PM +0800, gfree.w...@foxmail.com
> wrote:
> > > From: Gao Feng 
> > >
> > > The current call path of nf_ct_tcp_seqadj_set is the following.
> > >
> > > nfqnl_recv_verdict->ctnetlink_glue_hook->ctnetlink_glue_seqadj
> > > ->nf_ct_tcp_seqadj_set.
> > >
> > > It couldn't make sure the TCP header is in the linear data part.
> > > So use the skb_header_pointer instead of the current codes.
> > >
> > > BTW, the nf_ct_tcp_seqadj_set is one external function of netfilter
> > > which works in the network layer, it should not assume the transport
> > > header is in the linear data.
> > >
> > > Signed-off-by: Gao Feng 
> > > ---
> > >  net/netfilter/nf_conntrack_seqadj.c | 5 -
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/net/netfilter/nf_conntrack_seqadj.c
> > > b/net/netfilter/nf_conntrack_seqadj.c
> > > index ef7063e..80394ab 100644
> > > --- a/net/netfilter/nf_conntrack_seqadj.c
> > > +++ b/net/netfilter/nf_conntrack_seqadj.c
> > > @@ -61,11 +61,14 @@ void nf_ct_tcp_seqadj_set(struct sk_buff *skb,
> > > s32 off)
> > >  {
> > >   const struct tcphdr *th;
> > > + struct tcphdr tcph;
> > >
> > >   if (nf_ct_protonum(ct) != IPPROTO_TCP)
> > >   return;
> > >
> > > - th = (struct tcphdr *)(skb_network_header(skb) + ip_hdrlen(skb));
> > > + th = skb_header_pointer(skb, ip_hdrlen(skb), sizeof(tcph), &tcph);
> > > + if (!th)
> > > + return;
> >
> > This fix is required since your recent upgrade to check for 4 bytes
> > instead of 8 bytes from conntrack. Please confirm this.
> 
> Do you mean the commit e5e693ab49a95e1994979972eea224eefa81eba9
> "netfilter: conntrack: Only need first 4 bytes to get l4proto ports"?
> Sorry, I could not figure out why it would break the rule, and cause
issue.
> 
> Let's look at one change for TCP of that commit as following
> 
> diff --git a/net/netfilter/nf_conntrack_proto_tcp.c
> b/net/netfilter/nf_conntrack_proto_tcp.c
> index 70c8381..4abe9e1 100644
> --- a/net/netfilter/nf_conntrack_proto_tcp.c
> +++ b/net/netfilter/nf_conntrack_proto_tcp.c
> @@ -282,8 +282,8 @@ static bool tcp_pkt_to_tuple(const struct sk_buff
*skb,
> unsigned int dataoff,
> const struct tcphdr *hp;
> struct tcphdr _hdr;
> 
> -   /* Actually only need first 8 bytes. */
> -   hp = skb_header_pointer(skb, dataoff, 8, &_hdr);
> +   /* Actually only need first 4 bytes to get ports. */
> +   hp = skb_header_pointer(skb, dataoff, 4, &_hdr);
> if (hp == NULL)
> return false;
> 
> The skb_header_pointer only copies the data, not make the data linear.
> So I think it is ok after 4 bytes instead of 8 bytes.
> It only affect the local variable, not the skb.
> 
> >
> > If so, it would be good to review the NAT code to see if there are
> > more spots that are not broken since that change...
> 
> I also check the other codes of Netfilter.
> 
> grep -in skb_network_header * in nf/net/netfilter and
nf/net/ipv4(6)/netfilter/
> nf_nat_helper.c:41: data = skb_network_header(skb) + dataoff;
> nf_tables_core.c:99:ptr = skb_network_header(skb) +
> pkt->xt.thoff;
> xt_TCPMSS.c:104:tcph = (struct tcphdr *)(skb_network_header(skb) +
> tcphoff);
> xt_TCPMSS.c:163:tcph = (struct tcphdr
> *)(skb_network_header(skb) + tcphoff);
> xt_TCPOPTSTRIP.c:54:tcph = (struct tcphdr *)(skb_network_header(skb) +
> tcphoff);
> arpt_mangle.c:23:   arpptr = skb_network_header(skb) + sizeof(*arp);
> 
> all of them make sure the skb is linear, except the nf_tables_core.c which
use
> the skb_tail_pointer to exclude the non-linear data.
> 
> So there is only nf_ct_tcp_seqadj_set which need fixed.
> 
> Best Regards
> Feng

Sorry, please ignore this email. 
I reply the conversation with wrong email account which Outlook select it by
default.
I have replied again with the right email account.

Best 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 1/1] netfilter: seqadj: Fix possible non-linear data access for TCP header

2017-04-10 Thread Gao Feng
Hi Pablo,

> -Original Message-
> From: Pablo Neira Ayuso [mailto:pa...@netfilter.org]
> Sent: Monday, April 10, 2017 8:07 PM
> To: gfree.w...@foxmail.com
> Cc: netfilter-devel@vger.kernel.org; Gao Feng 
> Subject: Re: [PATCH nf 1/1] netfilter: seqadj: Fix possible non-linear
data access
> for TCP header
> 
> On Mon, Apr 10, 2017 at 06:36:03PM +0800, gfree.w...@foxmail.com wrote:
> > From: Gao Feng 
> >
> > The current call path of nf_ct_tcp_seqadj_set is the following.
> >
> > nfqnl_recv_verdict->ctnetlink_glue_hook->ctnetlink_glue_seqadj
> > ->nf_ct_tcp_seqadj_set.
> >
> > It couldn't make sure the TCP header is in the linear data part.
> > So use the skb_header_pointer instead of the current codes.
> >
> > BTW, the nf_ct_tcp_seqadj_set is one external function of netfilter
> > which works in the network layer, it should not assume the transport
> > header is in the linear data.
> >
> > Signed-off-by: Gao Feng 
> > ---
> >  net/netfilter/nf_conntrack_seqadj.c | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/netfilter/nf_conntrack_seqadj.c
> > b/net/netfilter/nf_conntrack_seqadj.c
> > index ef7063e..80394ab 100644
> > --- a/net/netfilter/nf_conntrack_seqadj.c
> > +++ b/net/netfilter/nf_conntrack_seqadj.c
> > @@ -61,11 +61,14 @@ void nf_ct_tcp_seqadj_set(struct sk_buff *skb,
> >   s32 off)
> >  {
> > const struct tcphdr *th;
> > +   struct tcphdr tcph;
> >
> > if (nf_ct_protonum(ct) != IPPROTO_TCP)
> > return;
> >
> > -   th = (struct tcphdr *)(skb_network_header(skb) + ip_hdrlen(skb));
> > +   th = skb_header_pointer(skb, ip_hdrlen(skb), sizeof(tcph), &tcph);
> > +   if (!th)
> > +   return;
> 
> This fix is required since your recent upgrade to check for 4 bytes
instead of 8
> bytes from conntrack. Please confirm this.

Do you mean the commit e5e693ab49a95e1994979972eea224eefa81eba9 "netfilter:
conntrack: Only need first 4 bytes to get l4proto ports"?
Sorry, I could not figure out why it would break the rule, and cause issue.

Let's look at one change for TCP of that commit as following

diff --git a/net/netfilter/nf_conntrack_proto_tcp.c
b/net/netfilter/nf_conntrack_proto_tcp.c
index 70c8381..4abe9e1 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -282,8 +282,8 @@ static bool tcp_pkt_to_tuple(const struct sk_buff *skb,
unsigned int dataoff,
const struct tcphdr *hp;
struct tcphdr _hdr;

-   /* Actually only need first 8 bytes. */
-   hp = skb_header_pointer(skb, dataoff, 8, &_hdr);
+   /* Actually only need first 4 bytes to get ports. */
+   hp = skb_header_pointer(skb, dataoff, 4, &_hdr);
if (hp == NULL)
return false;

The skb_header_pointer only copies the data, not make the data linear.
So I think it is ok after 4 bytes instead of 8 bytes.
It only affect the local variable, not the skb.

> 
> If so, it would be good to review the NAT code to see if there are more
spots
> that are not broken since that change...

I also check the other codes of Netfilter.

grep -in skb_network_header * in nf/net/netfilter and
nf/net/ipv4(6)/netfilter/
nf_nat_helper.c:41: data = skb_network_header(skb) + dataoff;
nf_tables_core.c:99:ptr = skb_network_header(skb) +
pkt->xt.thoff;
xt_TCPMSS.c:104:tcph = (struct tcphdr *)(skb_network_header(skb) +
tcphoff);
xt_TCPMSS.c:163:tcph = (struct tcphdr
*)(skb_network_header(skb) + tcphoff);
xt_TCPOPTSTRIP.c:54:tcph = (struct tcphdr *)(skb_network_header(skb) +
tcphoff);
arpt_mangle.c:23:   arpptr = skb_network_header(skb) + sizeof(*arp);

all of them make sure the skb is linear, except the nf_tables_core.c which
use the skb_tail_pointer to exclude the non-linear data.

So there is only nf_ct_tcp_seqadj_set which need fixed.

Best 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


[PATCH nf-next 1/1] netfilter: SYNPROXY: Return NF_STOLEN instead of NF_DROP during handshaking

2017-04-10 Thread gfree . wind
From: Gao Feng 

Current SYNPROXY codes return NF_DROP during normal TCP handshaking,
it is not friendly to caller. Because the nf_hook_slow would treat
the NF_DROP as an error, and return -EPERM.
As a result, it may cause the top caller think it meets one error.

So use NF_STOLEN instead of NF_DROP now because there is no error
happened indeed, and free the skb directly.

Signed-off-by: Gao Feng 
---
 net/ipv4/netfilter/ipt_SYNPROXY.c  | 7 ---
 net/ipv6/netfilter/ip6t_SYNPROXY.c | 6 --
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_SYNPROXY.c 
b/net/ipv4/netfilter/ipt_SYNPROXY.c
index 3240a26..9ed80d4 100644
--- a/net/ipv4/netfilter/ipt_SYNPROXY.c
+++ b/net/ipv4/netfilter/ipt_SYNPROXY.c
@@ -293,12 +293,13 @@
  XT_SYNPROXY_OPT_ECN);
 
synproxy_send_client_synack(net, skb, th, &opts);
-   return NF_DROP;
-
+   consume_skb(skb);
+   return NF_STOLEN;
} else if (th->ack && !(th->fin || th->rst || th->syn)) {
/* ACK from client */
synproxy_recv_client_ack(net, skb, th, &opts, ntohl(th->seq));
-   return NF_DROP;
+   consume_skb(skb);
+   return NF_STOLEN;
}
 
return XT_CONTINUE;
diff --git a/net/ipv6/netfilter/ip6t_SYNPROXY.c 
b/net/ipv6/netfilter/ip6t_SYNPROXY.c
index 4ef1ddd..d5d5725 100644
--- a/net/ipv6/netfilter/ip6t_SYNPROXY.c
+++ b/net/ipv6/netfilter/ip6t_SYNPROXY.c
@@ -307,12 +307,14 @@
  XT_SYNPROXY_OPT_ECN);
 
synproxy_send_client_synack(net, skb, th, &opts);
-   return NF_DROP;
+   consume_skb(skb);
+   return NF_STOLEN;
 
} else if (th->ack && !(th->fin || th->rst || th->syn)) {
/* ACK from client */
synproxy_recv_client_ack(net, skb, th, &opts, ntohl(th->seq));
-   return NF_DROP;
+   consume_skb(skb);
+   return NF_STOLEN;
}
 
return XT_CONTINUE;
-- 
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