netdev - re: netfilter: nf_conntrack: there maybe a bug in __nf_conntrack_confirm, when it race against get_next

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <01dd01cffa56$bdb10460$39130d20$@gmail.com>
Date:	Fri, 7 Nov 2014 14:47:32 +0800
From:	"Bill Bonaparte" <programme110@...il.com>
To:	"'Jesper Dangaard Brouer'" <brouer@...hat.com>
Cc:	<fw@...len.de>, <linux-kernel@...r.kernel.org>,
	"'Pablo Neira Ayuso'" <pablo@...filter.org>,
	"'Patrick McHardy'" <kaber@...sh.net>, <kadlec@...ckhole.kfki.hu>,
	<davem@...emloft.net>, "'Changli Gao'" <xiaosuo@...il.com>,
	"'Andrey Vagin'" <avagin@...nvz.org>,
	<netfilter-devel@...r.kernel.org>, <netdev@...r.kernel.org>
Subject: re: netfilter: nf_conntrack: there maybe a bug in __nf_conntrack_confirm, when it race against get_next_corpse


On Tue, 6 Nov 2014 21:01:00 
"Jesper" <brouter@...hat.com> wrote:
>There is several issues with your submission.  I'll take care of
resubmitting a patch in your name (so you will get credit in the git log).
>
>If you care to know, issues are:
>1. you are not sending to the appropriate mailing lists,  2. patch is as an
attachment (should be inlined),  3. the patch have style and white-space
issues.

Thanks, Jesper. This is my first time to submit a patch, not know much about
the rules.  I will get it soon.

>> if there is a race at operating ct->status, there will be in 
>> alternative
>> case:
>> 1) IPS_DYING bit which set in get_next_corpse override other bits (e.g.
>> IPS_SRC_NAT_DONE_BIT), or
>> 2) other bits (e.g. IPS_SRC_NAT_DONE_BIT) which set in 
>> nf_nat_setup_info override IPS_DYING bit.

> Notice the set_bit() is atomic, so we don't have these issues (of bits
getting overridden).

In most cases, we do the atomic operation on ct->status (with set_bit), but
in function nf_nat_setup_info, we
assume that unconfirmed ct is always holded by current cpu, and has no race
against other cpus, so we don't
use set_bit.  
the following code is extracted from the nf_nat_setup_info:
/* Non-atomic: we own this at the moment. */
  if (maniptype == NF_NAT_MANIP_SRC)
	ct->status |= IPS_SRC_NAT;
  else
	ct->status |= IPS_DST_NAT;

--
Best regards,
  Bill Bonaparte


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html