On Tue, Feb 22, 2022 at 7:35 AM Pavel Machek pavel@ucw.cz wrote:
Hi!
syzbot reported that two threads might write over agg_select_timer at the same time. Make agg_select_timer atomic to fix the races.
Ok, but:
--- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -249,7 +249,7 @@ static inline int __check_agg_selection_ if (bond == NULL) return 0;
return BOND_AD_INFO(bond).agg_select_timer ? 1 : 0;
return atomic_read(&BOND_AD_INFO(bond).agg_select_timer) ? 1 : 0;
}
This could probably use !!.
Probably... I chose to not change code style in a bug fix.
+static bool bond_agg_timer_advance(struct bonding *bond) +{
int val, nval;
while (1) {
val = atomic_read(&BOND_AD_INFO(bond).agg_select_timer);
if (!val)
return false;
nval = val - 1;
if (atomic_cmpxchg(&BOND_AD_INFO(bond).agg_select_timer,
val, nval) == val)
break;
}
return nval == 0;
+}
This should really be atomic_dec_if_positive, no?
SGTM, please send a patch, thank you.