netdev - Re: [PATCH RESEND] net/sctp: always initialise sctp_ht_iter::start

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20160813.151050.1364196940604628961.davem@davemloft.net>
Date:	Sat, 13 Aug 2016 15:10:50 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	vegard.nossum@...cle.com
Cc:	netdev@...r.kernel.org, linux-sctp@...r.kernel.org,
	lucien.xin@...il.com, herbert@...dor.apana.org.au,
	ebiederm@...ssion.com, marcelo.leitner@...il.com,
	stable@...r.kernel.org
Subject: Re: [PATCH RESEND] net/sctp: always initialise
 sctp_ht_iter::start_fail

From: Vegard Nossum <vegard.nossum@...cle.com>
Date: Fri, 12 Aug 2016 09:50:51 +0200

> sctp_transport_seq_start() does not currently clear iter->start_fail on
> success, but relies on it being zero when it is allocated (by
> seq_open_net()).
> 
> This can be a problem in the following sequence:
> 
>     open() // allocates iter (and implicitly sets iter->start_fail = 0)
>     read()
>      - iter->start() // fails and sets iter->start_fail = 1
>      - iter->stop() // doesn't call sctp_transport_walk_stop() (correct)
>     read() again
>      - iter->start() // succeeds, but doesn't change iter->start_fail
>      - iter->stop() // doesn't call sctp_transport_walk_stop() (wrong)
> 
> We should initialize sctp_ht_iter::start_fail to zero if ->start()
> succeeds, otherwise it's possible that we leave an old value of 1 there,
> which will cause ->stop() to not call sctp_transport_walk_stop(), which
> causes all sorts of problems like not calling rcu_read_unlock() (and
> preempt_enable()), eventually leading to more warnings like this:
 ...
> Notice that this is a subtly different stacktrace from the one in commit
> 5fc382d875 ("net/sctp: terminate rhashtable walk correctly").
> 
> Cc: Xin Long <lucien.xin@...il.com>
> Cc: Herbert Xu <herbert@...dor.apana.org.au>
> Cc: Eric W. Biederman <ebiederm@...ssion.com>
> Cc: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
> Signed-off-by: Vegard Nossum <vegard.nossum@...cle.com>

Applied and queued up for -stable, thanks.