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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 29 Dec 2019 17:47:46 +0000 From: Vlad Buslov <vladbu@...lanox.com> To: Davide Caratti <dcaratti@...hat.com> CC: "David S. Miller" <davem@...emloft.net>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, Vlad Buslov <vladbu@...lanox.com>, Jamal Hadi Salim <jhs@...atatu.com>, Cong Wang <xiyou.wangcong@...il.com>, Jiri Pirko <jiri@...nulli.us> 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 <dcaratti@...hat.com> 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 <jhs@...atatu.com> > Suggested-by: Vlad Buslov <vladbu@...lanox.com> > Signed-off-by: Davide Caratti <dcaratti@...hat.com> > --- Thanks again, Davide! Reviewed-by: Vlad Buslov <vladbu@...lanox.com>
Powered by blists - more mailing lists