| Message ID | 20201102143828.5286-1-menglong8.dong@gmail.com |
|---|---|
| State | Rejected |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: bridge: disable multicast while delete bridge | expand |
| Context | Check | Description |
|---|---|---|
| jkicinski/cover_letter | success | Link |
| jkicinski/fixes_present | success | Link |
| jkicinski/patch_count | success | Link |
| jkicinski/tree_selection | success | Guessed tree name to be net-next |
| jkicinski/subject_prefix | warning | Target tree name not specified in the subject |
| jkicinski/source_inline | success | Was 0 now: 0 |
| jkicinski/verify_signedoff | success | Link |
| jkicinski/module_param | success | Was 0 now: 0 |
| jkicinski/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/verify_fixes | success | Link |
| jkicinski/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 7 lines checked |
| jkicinski/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/header_inline | success | Link |
| jkicinski/stable | success | Stable not CCed |
On Mon, 2020-11-02 at 22:38 +0800, Menglong Dong wrote: > From: Menglong Dong <dong.menglong@zte.com.cn> > > This commit seems make no sense, as bridge is destroyed when > br_multicast_dev_del is called. > > In commit b1b9d366028f > ("bridge: move bridge multicast cleanup to ndo_uninit"), Xin Long > fixed the use-after-free panic in br_multicast_group_expired by > moving br_multicast_dev_del to ndo_uninit. However, that patch is > not applied to 4.4.X, and the bug exists. > > Fix that bug by disabling multicast in br_multicast_dev_del for > 4.4.X, and there is no harm for other branches. > > Signed-off-by: Menglong Dong <dong.menglong@zte.com.cn> > --- > net/bridge/br_multicast.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c > index eae898c3cff7..9992fdff2951 100644 > --- a/net/bridge/br_multicast.c > +++ b/net/bridge/br_multicast.c > @@ -3369,6 +3369,7 @@ void br_multicast_dev_del(struct net_bridge *br) > hlist_for_each_entry_safe(mp, tmp, &br->mdb_list, mdb_node) > br_multicast_del_mdb_entry(mp); > hlist_move_list(&br->mcast_gc_list, &deleted_head); > + br_opt_toggle(br, BROPT_MULTICAST_ENABLED, false); > spin_unlock_bh(&br->multicast_lock); > > br_multicast_gc(&deleted_head); This doesn't make any sense. It doesn't fix anything. If 4.4 has a problem then the relevant patches should get backported to it. We don't add random changes to fix older releases. Cheers, Nik Nacked-by: Nikolay Aleksandrov <nikolay@nvidia.com>
Dear Nik, On Wed, Nov 4, 2020 at 12:26 AM Nikolay Aleksandrov <nikolay@nvidia.com> wrote: > > On Mon, 2020-11-02 at 22:38 +0800, Menglong Dong wrote: > > From: Menglong Dong <dong.menglong@zte.com.cn> > > > > This commit seems make no sense, as bridge is destroyed when > > br_multicast_dev_del is called. > > > > In commit b1b9d366028f > > ("bridge: move bridge multicast cleanup to ndo_uninit"), Xin Long > > fixed the use-after-free panic in br_multicast_group_expired by > > moving br_multicast_dev_del to ndo_uninit. However, that patch is > > not applied to 4.4.X, and the bug exists. > > > > Fix that bug by disabling multicast in br_multicast_dev_del for > > 4.4.X, and there is no harm for other branches. > > > > Signed-off-by: Menglong Dong <dong.menglong@zte.com.cn> > > --- > > net/bridge/br_multicast.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c > > index eae898c3cff7..9992fdff2951 100644 > > --- a/net/bridge/br_multicast.c > > +++ b/net/bridge/br_multicast.c > > @@ -3369,6 +3369,7 @@ void br_multicast_dev_del(struct net_bridge *br) > > hlist_for_each_entry_safe(mp, tmp, &br->mdb_list, mdb_node) > > br_multicast_del_mdb_entry(mp); > > hlist_move_list(&br->mcast_gc_list, &deleted_head); > > + br_opt_toggle(br, BROPT_MULTICAST_ENABLED, false); > > spin_unlock_bh(&br->multicast_lock); > > > > br_multicast_gc(&deleted_head); > > This doesn't make any sense. It doesn't fix anything. > If 4.4 has a problem then the relevant patches should get backported to it. > We don't add random changes to fix older releases. > > Cheers, > Nik > > Nacked-by: Nikolay Aleksandrov <nikolay@nvidia.com> Thanks for your patient explanation, and I see it now~ Cheers, Menglong Dong
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c index eae898c3cff7..9992fdff2951 100644 --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -3369,6 +3369,7 @@ void br_multicast_dev_del(struct net_bridge *br) hlist_for_each_entry_safe(mp, tmp, &br->mdb_list, mdb_node) br_multicast_del_mdb_entry(mp); hlist_move_list(&br->mcast_gc_list, &deleted_head); + br_opt_toggle(br, BROPT_MULTICAST_ENABLED, false); spin_unlock_bh(&br->multicast_lock); br_multicast_gc(&deleted_head);