* [PATCH] bonding: init notify_work earlier to avoid uninitialized use
@ 2021-05-17 14:13 Johannes Berg
2021-05-17 15:39 ` Jay Vosburgh
2021-05-17 22:30 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Johannes Berg @ 2021-05-17 14:13 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, Johannes Berg,
syzbot+bfda097c12a00c8cae67
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>
---
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] bonding: init notify_work earlier to avoid uninitialized use
2021-05-17 14:13 [PATCH] bonding: init notify_work earlier to avoid uninitialized use Johannes Berg
@ 2021-05-17 15:39 ` Jay Vosburgh
2021-05-17 22:30 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Jay Vosburgh @ 2021-05-17 15:39 UTC (permalink / raw)
To: Johannes Berg
Cc: netdev, Veaceslav Falico, Andy Gospodarek, Johannes Berg,
syzbot+bfda097c12a00c8cae67
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] bonding: init notify_work earlier to avoid uninitialized use
2021-05-17 14:13 [PATCH] bonding: init notify_work earlier to avoid uninitialized use Johannes Berg
2021-05-17 15:39 ` Jay Vosburgh
@ 2021-05-17 22:30 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-05-17 22:30 UTC (permalink / raw)
To: Johannes Berg
Cc: netdev, j.vosburgh, vfalico, andy, johannes.berg,
syzbot+bfda097c12a00c8cae67
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-05-17 22:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-17 14:13 [PATCH] bonding: init notify_work earlier to avoid uninitialized use Johannes Berg
2021-05-17 15:39 ` Jay Vosburgh
2021-05-17 22:30 ` patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.