| Message ID | 20210517161335.e40fea7f895a.I8b8487a9c0b8f54716cf44fdae02185381b1f64e@changeid (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 35d96e631860226d5dc4de0fad0a415362ec2457 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | bonding: init notify_work earlier to avoid uninitialized use | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | warning | 2 maintainers not CCed: davem@davemloft.net kuba@kernel.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 5 this patch: 5 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 14 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 5 this patch: 5 |
| netdev/header_inline | success | Link |
Johannes Berg <johannes@sipsolutions.net> wrote: >From: Johannes Berg <johannes.berg@intel.com> > >If bond_kobj_init() or later kzalloc() in bond_alloc_slave() fail, >then we call kobject_put() on the slave->kobj. This in turn calls >the release function slave_kobj_release() which will always try to >cancel_delayed_work_sync(&slave->notify_work), which shouldn't be >done on an uninitialized work struct. > >Always initialize the work struct earlier to avoid problems here. > >Syzbot bisected this down to a completely pointless commit, some >fault injection may have been at work here that caused the alloc >failure in the first place, which may interact badly with bisect. > >Reported-by: syzbot+bfda097c12a00c8cae67@syzkaller.appspotmail.com >Signed-off-by: Johannes Berg <johannes.berg@intel.com> Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com> >--- > drivers/net/bonding/bond_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >index 20bbda1b36e1..c5a646d06102 100644 >--- a/drivers/net/bonding/bond_main.c >+++ b/drivers/net/bonding/bond_main.c >@@ -1526,6 +1526,7 @@ static struct slave *bond_alloc_slave(struct bonding *bond, > > slave->bond = bond; > slave->dev = slave_dev; >+ INIT_DELAYED_WORK(&slave->notify_work, bond_netdev_notify_work); > > if (bond_kobj_init(slave)) > return NULL; >@@ -1538,7 +1539,6 @@ static struct slave *bond_alloc_slave(struct bonding *bond, > return NULL; > } > } >- INIT_DELAYED_WORK(&slave->notify_work, bond_netdev_notify_work); > > return slave; > } >-- >2.31.1 >
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Mon, 17 May 2021 16:13:35 +0200 you wrote: > From: Johannes Berg <johannes.berg@intel.com> > > If bond_kobj_init() or later kzalloc() in bond_alloc_slave() fail, > then we call kobject_put() on the slave->kobj. This in turn calls > the release function slave_kobj_release() which will always try to > cancel_delayed_work_sync(&slave->notify_work), which shouldn't be > done on an uninitialized work struct. > > [...] Here is the summary with links: - bonding: init notify_work earlier to avoid uninitialized use https://git.kernel.org/netdev/net/c/35d96e631860 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 20bbda1b36e1..c5a646d06102 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1526,6 +1526,7 @@ static struct slave *bond_alloc_slave(struct bonding *bond, slave->bond = bond; slave->dev = slave_dev; + INIT_DELAYED_WORK(&slave->notify_work, bond_netdev_notify_work); if (bond_kobj_init(slave)) return NULL; @@ -1538,7 +1539,6 @@ static struct slave *bond_alloc_slave(struct bonding *bond, return NULL; } } - INIT_DELAYED_WORK(&slave->notify_work, bond_netdev_notify_work); return slave; }