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  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sun, 29 Dec 2019 17:47:46 +0000
From:   Vlad Buslov <>
To:     Davide Caratti <>
CC:     "David S. Miller" <>,
        "" <>,
        Vlad Buslov <>,
        Jamal Hadi Salim <>,
        Cong Wang <>,
        Jiri Pirko <>
Subject: Re: [PATCH net] net/sched: add delete_empty() to filters and use it
 in cls_flower

On Sat 28 Dec 2019 at 17:36, Davide Caratti <> wrote:
> Revert "net/sched: cls_u32: fix refcount leak in the error path of
> u32_change()", and fix the u32 refcount leak in a more generic way that
> preserves the semantic of rule dumping.
> On tc filters that don't support lockless insertion/removal, there is no
> need to guard against concurrent insertion when a removal is in progress.
> Therefore, for most of them we can avoid a full walk() when deleting, and
> just decrease the refcount, like it was done on older Linux kernels.
> This fixes situations where walk() was wrongly detecting a non-empty
> filter, like it happened with cls_u32 in the error path of change(), thus
> leading to failures in the following tdc selftests:
>  6aa7: (filter, u32) Add/Replace u32 with source match and invalid indev
>  6658: (filter, u32) Add/Replace u32 with custom hash table and invalid handle
>  74c2: (filter, u32) Add/Replace u32 filter with invalid hash table id
> On cls_flower, and on (future) lockless filters, this check is necessary:
> move all the check_empty() logic in a callback so that each filter
> can have its own implementation. For cls_flower, it's sufficient to check
> if no IDRs have been allocated.
> This reverts commit 275c44aa194b7159d1191817b20e076f55f0e620.
> Changes since v1:
>  - document the need for delete_empty() when TCF_PROTO_OPS_DOIT_UNLOCKED
>    is used, thanks to Vlad Buslov
>  - implement delete_empty() without doing fl_walk(), thanks to Vlad Buslov
>  - squash revert and new fix in a single patch, to be nice with bisect
>    tests that run tdc on u32 filter, thanks to Dave Miller
> Fixes: 275c44aa194b ("net/sched: cls_u32: fix refcount leak in the error path of u32_change()")
> Fixes: 6676d5e416ee ("net: sched: set dedicated tcf_walker flag when tp is empty")
> Suggested-by: Jamal Hadi Salim <>
> Suggested-by: Vlad Buslov <>
> Signed-off-by: Davide Caratti <>
> ---

Thanks again, Davide!

Reviewed-by: Vlad Buslov <>

Powered by blists - more mailing lists