Re: KASAN, xt_TCPMSS finally found nasty use-after-free bug? 4.10.8

2017-04-02 Thread Eric Dumazet
On Sun, 2017-04-02 at 10:14 -0700, Eric Dumazet wrote: > Could that be that netfilter does not abort earlier if TCP header is > completely wrong ? > Yes, I wonder if this patch would be better, unless we replicate the th->doff sanity check in all netfilter modules dissecting TCP frames. diff

Re: KASAN, xt_TCPMSS finally found nasty use-after-free bug? 4.10.8

2017-04-02 Thread Eric Dumazet
On Sun, 2017-04-02 at 19:52 +0300, Denys Fedoryshchenko wrote: > On 2017-04-02 15:32, Eric Dumazet wrote: > > On Sun, 2017-04-02 at 15:25 +0300, Denys Fedoryshchenko wrote: > >> > */ > >> I will add also WARN_ON_ONCE(tcp_hdrlen >= 15 * 4) before, for > >> curiosity, if this condition are

Re: KASAN, xt_TCPMSS finally found nasty use-after-free bug? 4.10.8

2017-04-02 Thread Denys Fedoryshchenko
On 2017-04-02 15:32, Eric Dumazet wrote: On Sun, 2017-04-02 at 15:25 +0300, Denys Fedoryshchenko wrote: > */ I will add also WARN_ON_ONCE(tcp_hdrlen >= 15 * 4) before, for curiosity, if this condition are triggered. Is it fine like that? Sure. It didnt triggered WARN_ON, and with both

[PATCH 1/2] iptables: extensions: unnecessary cast on void pointer

2017-04-02 Thread Arushi Singhal
The following Coccinelle script was used to detect this: @r@ expression x; void* e; type T; identifier f; @@ ( *((T *)e) | ((T *)x)[...] | ((T*)x)->f | - (T*) e ) Signed-off-by: Arushi Singhal --- extensions/libarpt_mangle.c | 2 +-

[PATCH 2/2] iptables: iptables: unnecessary cast on void pointer

2017-04-02 Thread Arushi Singhal
The following Coccinelle script was used to detect this: @r@ expression x; void* e; type T; identifier f; @@ ( *((T *)e) | ((T *)x)[...] | ((T*)x)->f | - (T*) e ) Signed-off-by: Arushi Singhal --- iptables/nft-arp.c | 4 ++-- iptables/xtables-arp.c

Re: KASAN, xt_TCPMSS finally found nasty use-after-free bug? 4.10.8

2017-04-02 Thread Eric Dumazet
On Sun, 2017-04-02 at 15:25 +0300, Denys Fedoryshchenko wrote: > > */ > I will add also WARN_ON_ONCE(tcp_hdrlen >= 15 * 4) before, for > curiosity, if this condition are triggered. Is it fine like that? Sure. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in

Re: KASAN, xt_TCPMSS finally found nasty use-after-free bug? 4.10.8

2017-04-02 Thread Denys Fedoryshchenko
On 2017-04-02 15:19, Eric Dumazet wrote: On Sun, 2017-04-02 at 04:54 -0700, Eric Dumazet wrote: On Sun, 2017-04-02 at 13:45 +0200, Florian Westphal wrote: > Eric Dumazet wrote: > > - for (i = sizeof(struct tcphdr); i <= tcp_hdrlen - TCPOLEN_MSS; i += optlen(opt,

Re: KASAN, xt_TCPMSS finally found nasty use-after-free bug? 4.10.8

2017-04-02 Thread Eric Dumazet
On Sun, 2017-04-02 at 04:54 -0700, Eric Dumazet wrote: > On Sun, 2017-04-02 at 13:45 +0200, Florian Westphal wrote: > > Eric Dumazet wrote: > > > - for (i = sizeof(struct tcphdr); i <= tcp_hdrlen - TCPOLEN_MSS; i += > > > optlen(opt, i)) { > > > + for (i = sizeof(struct

Re: KASAN, xt_TCPMSS finally found nasty use-after-free bug? 4.10.8

2017-04-02 Thread Eric Dumazet
On Sun, 2017-04-02 at 13:45 +0200, Florian Westphal wrote: > Eric Dumazet wrote: > > - for (i = sizeof(struct tcphdr); i <= tcp_hdrlen - TCPOLEN_MSS; i += > > optlen(opt, i)) { > > + for (i = sizeof(struct tcphdr); i < tcp_hdrlen - TCPOLEN_MSS; i += > > optlen(opt,

Re: KASAN, xt_TCPMSS finally found nasty use-after-free bug? 4.10.8

2017-04-02 Thread Denys Fedoryshchenko
On 2017-04-02 14:45, Florian Westphal wrote: Eric Dumazet wrote: - for (i = sizeof(struct tcphdr); i <= tcp_hdrlen - TCPOLEN_MSS; i += optlen(opt, i)) { + for (i = sizeof(struct tcphdr); i < tcp_hdrlen - TCPOLEN_MSS; i += optlen(opt, i)) { if (opt[i]

Re: KASAN, xt_TCPMSS finally found nasty use-after-free bug? 4.10.8

2017-04-02 Thread Florian Westphal
Eric Dumazet wrote: > - for (i = sizeof(struct tcphdr); i <= tcp_hdrlen - TCPOLEN_MSS; i += > optlen(opt, i)) { > + for (i = sizeof(struct tcphdr); i < tcp_hdrlen - TCPOLEN_MSS; i += > optlen(opt, i)) { > if (opt[i] == TCPOPT_MSS && opt[i+1] ==

Re: KASAN, xt_TCPMSS finally found nasty use-after-free bug? 4.10.8

2017-04-02 Thread Eric Dumazet
On Sun, 2017-04-02 at 10:43 +0300, Denys Fedoryshchenko wrote: > Repost, due being sleepy missed few important points. > > I am searching reasons of crashes for multiple conntrack enabled > servers, usually they point to conntrack, but i suspect use after free > might be somewhere else, > so i

[PATCH nf] netfilter: nf_ct_expect: use proper RCU list traversal/update APIs

2017-04-02 Thread Liping Zhang
From: Liping Zhang We should use proper RCU list APIs to manipulate help->expectations, as we can dump the conntrack's expectations via nfnetlink, i.e. in ctnetlink_exp_ct_dump_table(), where only rcu_read_lock is acquired. So for list traversal, use

[PATCH nf] netfilter: ctnetlink: skip dumping expect when nfct_help(ct) is NULL

2017-04-02 Thread Liping Zhang
From: Liping Zhang For IPCTNL_MSG_EXP_GET, if the CTA_EXPECT_MASTER attr is specified, then the NLM_F_DUMP request will dump the expectations related to this connection tracking. But we forget to check whether the conntrack has nf_conn_help or not, so if nfct_help(ct) is

[PATCH nf] netfilter: make it safer during the inet6_dev->addr_list traversal

2017-04-02 Thread Liping Zhang
From: Liping Zhang inet6_dev->addr_list is protected by inet6_dev->lock, so only using rcu_read_lock is not enough, we should acquire read_lock_bh(>lock) before the inet6_dev->addr_list traversal. Signed-off-by: Liping Zhang ---

[PATCH] iptables: iptables: Remove exceptional & on function name

2017-04-02 Thread Arushi Singhal
Remove & from function pointers to conform to the style found elsewhere in the file. Done using the following semantic patch // @r@ identifier f; @@ f(...) { ... } @@ identifier r.f; @@ - + f // Signed-off-by: Arushi Singhal --- iptables/ip6tables-save.c |

[PATCH 1/4] net: netfilter:Remove exceptional & on function name

2017-04-02 Thread Arushi Singhal
Remove & from function pointers to conform to the style found elsewhere in the file. Done using the following semantic patch // @r@ identifier f; @@ f(...) { ... } @@ identifier r.f; @@ - + f // Signed-off-by: Arushi Singhal ---

[PATCH 3/4] net: ipv4: Remove exceptional & on function name

2017-04-02 Thread Arushi Singhal
Remove & from function pointers to conform to the style found elsewhere in the file. Done using the following semantic patch // @r@ identifier f; @@ f(...) { ... } @@ identifier r.f; @@ - + f // Signed-off-by: Arushi Singhal ---

[PATCH 4/4] net: ipv6: Remove exceptional & on function name

2017-04-02 Thread Arushi Singhal
Remove & from function pointers to conform to the style found elsewhere in the file. Done using the following semantic patch // @r@ identifier f; @@ f(...) { ... } @@ identifier r.f; @@ - + f // Signed-off-by: Arushi Singhal ---

[PATCH 2/4] net: bridge: Remove exceptional & on function name

2017-04-02 Thread Arushi Singhal
Remove & from function pointers to conform to the style found elsewhere in the file. Done using the following semantic patch // @r@ identifier f; @@ f(...) { ... } @@ identifier r.f; @@ - + f // Signed-off-by: Arushi Singhal ---

KASAN, xt_TCPMSS finally found nasty use-after-free bug? 4.10.8

2017-04-02 Thread Denys Fedoryshchenko
Repost, due being sleepy missed few important points. I am searching reasons of crashes for multiple conntrack enabled servers, usually they point to conntrack, but i suspect use after free might be somewhere else, so i tried to enable KASAN. And seems i got something after few hours, and it

[PATCH 2/2] iptables: iptables: Constify option struct

2017-04-02 Thread Arushi Singhal
The structs of the type option are not modified anywhere. Signed-off-by: Arushi Singhal --- iptables/ip6tables.c| 2 +- iptables/iptables-xml.c | 2 +- iptables/xtables-arp.c | 2 +- iptables/xtables-eb.c | 2 +- iptables/xtables.c | 2 +- 5 files

[PATCH 1/2] extensions: libarpt_mangle.c : Constify option struct

2017-04-02 Thread Arushi Singhal
The struct arpmangle_opts of the type option is only used to initialise a field inside the xtables_target struct and is not modified anywhere. Signed-off-by: Arushi Singhal --- extensions/libarpt_mangle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)