[PATCH 1/2 nf] netfilter: seqadj: Fix some possible panics of seqadj when mem is exhausted
Subsystems:
netfilter , networking [general] , the rest
3681d
4 messages,
3 authors,
2016-09-16 · open the first message on its own page
From: Gao Feng <redacted>
When memory is exhausted, nfct_seqadj_ext_add may fail to add the seqadj
extension. But these interface functions nf_ct_seqadj_init and
nf_ct_seq_adjust don't check if they get the valid seqadj pointer by the
nfct_seqadj, while nf_ct_seqadj_set and nf_ct_seq_offset perform that
check. So the system would be panic when nfct_seqadj_ext_add failed.
Signed-off-by: Gao Feng <redacted>
---
net/netfilter/nf_conntrack_seqadj.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nf_conntrack_seqadj.c b/net/netfilter/nf_conntrack_seqadj.c
index dff0f0c..2a2fd0e 100644
--- a/net/netfilter/nf_conntrack_seqadj.c
+++ b/net/netfilter/nf_conntrack_seqadj.c @@ -16,9 +16,14 @@ int nf_ct_seqadj_init(struct nf_conn *ct, enum ip_conntrack_info ctinfo,
if ( off == 0 )
return 0 ;
+ seqadj = nfct_seqadj ( ct ); + if ( unlikely ( ! seqadj )) { + WARN_ONCE ( 1 , "Missing nfct_seqadj_ext_add() setup call \n " ); + return 0 ; + } + set_bit ( IPS_SEQ_ADJUST_BIT , & ct -> status );
- seqadj = nfct_seqadj ( ct ); this_way = & seqadj -> seq [ dir ];
this_way -> offset_before = off ;
this_way -> offset_after = off ; @@ -171,6 +176,11 @@ int nf_ct_seq_adjust(struct sk_buff *skb,
struct nf_ct_seqadj * this_way , * other_way ;
int res ;
+ if ( unlikely ( ! seqadj )) { + WARN_ONCE ( 1 , "Missing nfct_seqadj_ext_add() setup call \n " ); + return 0 ; + } + this_way = & seqadj -> seq [ dir ];
other_way = & seqadj -> seq [ ! dir ];
@@ -218,8 +228,10 @@ s32 nf_ct_seq_offset(const struct nf_conn *ct,
struct nf_conn_seqadj * seqadj = nfct_seqadj ( ct );
struct nf_ct_seqadj * this_way ;
- if ( ! seqadj ) + if ( unlikely ( ! seqadj )) { + WARN_ONCE ( 1 , "Missing nfct_seqadj_ext_add() setup call \n " ); return 0 ;
+ }
this_way = & seqadj -> seq [ dir ];
return after ( seq , this_way -> correction_pos ) ? --
1.9.1
Hi Feng,
2016-09-02 9:48 GMT+08:00 [off-list ref]: quoted hunk From: Gao Feng <redacted> @@ -171,6 +176,11 @@ int nf_ct_seq_adjust(struct sk_buff *skb,
struct nf_ct_seqadj *this_way, *other_way;
int res;
+ if (unlikely(!seqadj)) {
IPS_SEQ_ADJUST_BIT will be tested before we call nf_ct_seq_adjust(),
so I think seqadj
will never be NULL here.
quoted hunk + WARN_ONCE(1, "Missing nfct_seqadj_ext_add() setup call\n");+ return 0;+ }+ this_way = &seqadj->seq[dir];
other_way = &seqadj->seq[!dir];
@@ -218,8 +228,10 @@ s32 nf_ct_seq_offset(const struct nf_conn *ct,
struct nf_conn_seqadj *seqadj = nfct_seqadj(ct);
struct nf_ct_seqadj *this_way;
- if (!seqadj)+ if (unlikely(!seqadj)) {+ WARN_ONCE(1, "Missing nfct_seqadj_ext_add() setup call\n");
But in nf_ct_seq_offset, seqadj is likely to be NULL, see the function
call path:
tcp_packet->tcp_in_window->nf_ct_seq_offset, so WARN_ONCE seems unnecessary.
return 0;
+ }
Hi Liping,
On Fri, Sep 2, 2016 at 12:50 PM, Liping Zhang [off-list ref] wrote: Hi Feng,
2016-09-02 9:48 GMT+08:00 [off-list ref]: quoted From: Gao Feng <redacted> @@ -171,6 +176,11 @@ int nf_ct_seq_adjust(struct sk_buff *skb,
struct nf_ct_seqadj *this_way, *other_way;
int res;
+ if (unlikely(!seqadj)) {
IPS_SEQ_ADJUST_BIT will be tested before we call nf_ct_seq_adjust(),
so I think seqadj
will never be NULL here.
Thanks. I didn't check IPS_SEQ_ADJUST_BIT flag.
quoted + WARN_ONCE(1, "Missing nfct_seqadj_ext_add() setup call\n");+ return 0;+ }+ this_way = &seqadj->seq[dir];
other_way = &seqadj->seq[!dir];
@@ -218,8 +228,10 @@ s32 nf_ct_seq_offset(const struct nf_conn *ct,
struct nf_conn_seqadj *seqadj = nfct_seqadj(ct);
struct nf_ct_seqadj *this_way;
- if (!seqadj)+ if (unlikely(!seqadj)) {+ WARN_ONCE(1, "Missing nfct_seqadj_ext_add() setup call\n");
But in nf_ct_seq_offset, seqadj is likely to be NULL, see the function
call path:
tcp_packet->tcp_in_window->nf_ct_seq_offset, so WARN_ONCE seems unnecessary.
quoted return 0;
+ }
Get it. It could be NULL.
Then only nf_ct_seqadj_init is necessary to check seqadj.
I will update this patch.
Regards
Feng
FYI, we noticed the following commit:
https://github.com/0day-ci/linux fgao-ikuai8-com/netfilter-seqadj-Fix-some-possible-panics-of-seqadj-when-mem-is-exhausted/20160902-095727
commit 68263ddb4777cc996868498e3d56f616851966d2 ("netfilter: seqadj: Fix some possible panics of seqadj when mem is exhausted")
in testcase: boot
on test machine: qemu-system-x86_64 -enable-kvm -m 320M
caused below changes:
+------------------------------------------------------------------+------------+------------+
| | c73c248490 | 68263ddb47 |
+------------------------------------------------------------------+------------+------------+
| boot_successes | 7 | 0 |
| boot_failures | 7 | 14 |
| BUG:kernel_reboot-without-warning_in_test_stage | 7 | |
| WARNING:at_net/netfilter/nf_conntrack_seqadj.c:#nf_ct_seq_offset | 0 | 14 |
| calltrace:SyS_connect | 0 | 14 |
| calltrace:SyS_socketcall | 0 | 14 |
| invoked_oom-killer:gfp_mask=0x | 0 | 1 |
| Mem-Info | 0 | 1 |
+------------------------------------------------------------------+------------+------------+
[ 22.475640] e1000: eth0 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX
[ 22.602089] Kernel tests: Boot OK!
[ 23.465616] ------------[ cut here ]------------
[ 23.466477] WARNING: CPU: 0 PID: 1225 at net/netfilter/nf_conntrack_seqadj.c:232 nf_ct_seq_offset+0x7a/0x9a
[ 23.468458] Missing nfct_seqadj_ext_add() setup call
[ 23.469319] CPU: 0 PID: 1225 Comm: busybox Not tainted 4.8.0-rc2-00241-g68263dd #1
[ 23.470629] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Debian-1.8.2-1 04/01/2014
[ 23.472138] 00000000 8ca4db58 8ca4db28 8a045c76 8ca4db44 89e4a2a3 000000e8 8a97c7b6
[ 23.491476] 00000001 00000000 00000001 8ca4db60 89e4a2e7 00000009 00000000 8ca4db58
[ 23.493027] 8b2c55b0 8ca4db74 8ca4db80 8a97c7b6 8b2c55da 000000e8 8b2c55b0 8e358000
[ 23.494559] Call Trace:
[ 23.495013] [<8a045c76>] dump_stack+0x16/0x18
[ 23.495777] [<89e4a2a3>] __warn+0xaa/0xc1
[ 23.496500] [<8a97c7b6>] ? nf_ct_seq_offset+0x7a/0x9a
[ 23.497392] [<89e4a2e7>] warn_slowpath_fmt+0x2d/0x32
[ 23.498270] [<8a97c7b6>] nf_ct_seq_offset+0x7a/0x9a
[ 23.499134] [<8a97b2a3>] tcp_packet+0x63f/0xb93
[ 23.499924] [<89f39167>] ? cache_alloc_refill+0x203/0x80a
[ 23.500896] [<89e6863e>] ? preempt_count_sub+0x8f/0xd4
[ 23.501824] [<8a976d99>] ? nf_conntrack_in+0x2f9/0x53e
[ 23.502747] [<8a976dc2>] ? nf_conntrack_in+0x322/0x53e
[ 23.521594] [<89e6863e>] ? preempt_count_sub+0x8f/0xd4
[ 23.522508] [<8a976f5b>] nf_conntrack_in+0x4bb/0x53e
[ 23.523399] [<8a9e6a9a>] ipv4_conntrack_local+0x40/0x48
[ 23.524338] [<8a9729c5>] nf_iterate+0x3b/0x8e
[ 23.525128] [<8a972a5f>] nf_hook_slow+0x47/0xc7
[ 23.525925] [<8a9a47f1>] __ip_local_out+0xdb/0xea
[ 23.526766] [<8a9a3750>] ? ip_options_rcv_srr+0x30e/0x30e
[ 23.527785] [<8a9a4818>] ip_local_out+0x18/0x9e
[ 23.528600] [<8a9a4dd8>] ip_queue_xmit+0x395/0x41e
[ 23.529469] [<8a9b93cb>] tcp_transmit_skb+0x701/0x740
[ 23.530420] [<8a9bbcba>] tcp_connect+0x65a/0x6be
[ 23.531247] [<8a9bd9e3>] tcp_v4_connect+0x41d/0x45c
[ 23.532115] [<8a9d1a6c>] __inet_stream_connect+0x77/0x27e
[ 23.533107] [<89e6863e>] ? preempt_count_sub+0x8f/0xd4
[ 23.534018] [<89e4d8e1>] ? __local_bh_enable_ip+0xc4/0xea
[ 23.534949] [<8a9d1c9e>] inet_stream_connect+0x2b/0x3e
[ 23.535862] [<8a935255>] SyS_connect+0x77/0x9d
[ 23.536662] [<89f74959>] ? __fd_install+0x163/0x1c5
[ 23.538079] [<89e6863e>] ? preempt_count_sub+0x8f/0xd4
Thanks,
Xiaolong