netdev - Re: [PATCH net] net: sched: fix refcount imbalance in actions

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>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20150730.142116.2111905680725756228.davem@davemloft.net>
Date:	Thu, 30 Jul 2015 14:21:16 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	daniel@...earbox.net
Cc:	cwang@...pensource.com, ast@...mgrid.com, jhs@...atatu.com,
	netdev@...r.kernel.org
Subject: Re: [PATCH net] net: sched: fix refcount imbalance in actions

From: Daniel Borkmann <daniel@...earbox.net>
Date: Wed, 29 Jul 2015 23:35:25 +0200

> Since commit 55334a5db5cd ("net_sched: act: refuse to remove bound action
> outside"), we end up with a wrong reference count for a tc action.
 ...
> What happens is that in tcf_hash_check(), we check tcf_common for a given
> index and increase tcfc_refcnt and conditionally tcfc_bindcnt when we've
> found an existing action. Now there are the following cases:
> 
>   1) We do a late binding of an action. In that case, we leave the
>      tcfc_refcnt/tcfc_bindcnt increased and are done with the ->init()
>      handler. This is correctly handeled.
> 
>   2) We replace the given action, or we try to add one without replacing
>      and find out that the action at a specific index already exists
>      (thus, we go out with error in that case).
> 
> In case of 2), we have to undo the reference count increase from
> tcf_hash_check() in the tcf_hash_check() function. Currently, we fail to
> do so because of the 'tcfc_bindcnt > 0' check which bails out early with
> an -EPERM error.
> 
> Now, while commit 55334a5db5cd prevents 'tc actions del action ...' on an
> already classifier-bound action to drop the reference count (which could
> then become negative, wrap around etc), this restriction only accounts for
> invocations outside a specific action's ->init() handler.
> 
> One possible solution would be to add a flag thus we possibly trigger
> the -EPERM ony in situations where it is indeed relevant.
> 
> After the patch, above test cases have correct reference count again.
> 
> Fixes: 55334a5db5cd ("net_sched: act: refuse to remove bound action outside")
> Signed-off-by: Daniel Borkmann <daniel@...earbox.net>

Applied and queued up for -stable, thanks Daniel.
--
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