| 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 linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1283438186.2454.856.camel@edumazet-laptop>
Date: Thu, 02 Sep 2010 16:36:26 +0200
From: Eric Dumazet <eric.dumazet@...il.com>
To: Jean Delvare <jdelvare@...e.de>
Cc: Jay Vosburgh <fubar@...ibm.com>,
bonding-devel@...ts.sourceforge.net, netdev@...r.kernel.org,
Jiri Bohac <jbohac@...e.cz>
Subject: Re: [PATCH] bonding: Fix jiffies overflow problems (again)
Le jeudi 02 septembre 2010 à 16:19 +0200, Jean Delvare a écrit :
> From: Jiri Bohac <jbohac@...e.cz>
>
> The time_before_eq()/time_after_eq() functions operate on unsigned
> long and only work if the difference between the two compared values
> is smaller than half the range of unsigned long (31 bits on i386).
>
> Some of the variables (slave->jiffies, dev->trans_start, dev->last_rx)
> used by bonding store a copy of jiffies and may not be updated for a
> long time. With HZ=1000, time_before_eq()/time_after_eq() will start
> giving bad results after ~25 days.
>
> jiffies will never be before slave->jiffies, dev->trans_start,
> dev->last_rx by more than possibly a couple ticks caused by preemption
> of this code. This allows us to detect/prevent these overflows by
> replacing time_before_eq()/time_after_eq() with time_in_range().
>
> Signed-off-by: Jiri Bohac <jbohac@...e.cz>
> Signed-off-by: Jean Delvare <jdelvare@...e.de>
> ---
> drivers/net/bonding/bond_main.c | 48 +++++++++++++++++++++++++++-------------
> 1 file changed, 33 insertions(+), 15 deletions(-)
>
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2798,8 +2798,12 @@ void bond_loadbalance_arp_mon(struct wor
> */
> bond_for_each_slave(bond, slave, i) {
> if (slave->link != BOND_LINK_UP) {
> - if (time_before_eq(jiffies, dev_trans_start(slave->dev) + delta_in_ticks) &&
> - time_before_eq(jiffies, slave->dev->last_rx + delta_in_ticks)) {
> + if (time_in_range(jiffies,
> + dev_trans_start(slave->dev) - delta_in_ticks,
> + dev_trans_start(slave->dev) + delta_in_ticks) &&
since dev_trans_start() might be expensive, you probably should cache
its result.
> + time_in_range(jiffies,
> + slave->dev->last_rx - delta_in_ticks,
> + slave->dev->last_rx + delta_in_ticks)) {
>
> slave->link = BOND_LINK_UP;
> slave->state = BOND_STATE_ACTIVE;
> @@ -2827,8 +2831,12 @@ void bond_loadbalance_arp_mon(struct wor
> * when the source ip is 0, so don't take the link down
> * if we don't know our ip yet
> */
> - if (time_after_eq(jiffies, dev_trans_start(slave->dev) + 2*delta_in_ticks) ||
> - (time_after_eq(jiffies, slave->dev->last_rx + 2*delta_in_ticks))) {
> + if (!time_in_range(jiffies,
> + dev_trans_start(slave->dev) - delta_in_ticks,
> + dev_trans_start(slave->dev) + 2*delta_in_ticks) ||
> + (!time_in_range(jiffies,
> + slave->dev->last_rx - delta_in_ticks,
> + slave->dev->last_rx + 2*delta_in_ticks))) {
>
> slave->link = BOND_LINK_DOWN;
> slave->state = BOND_STATE_BACKUP;
> @@ -2888,8 +2896,10 @@ static int bond_ab_arp_inspect(struct bo
> slave->new_link = BOND_LINK_NOCHANGE;
>
> if (slave->link != BOND_LINK_UP) {
> - if (time_before_eq(jiffies, slave_last_rx(bond, slave) +
> - delta_in_ticks)) {
> + if (time_in_range(jiffies,
> + slave_last_rx(bond, slave) - delta_in_ticks,
> + slave_last_rx(bond, slave) + delta_in_ticks)) {
> +
> slave->new_link = BOND_LINK_UP;
> commit++;
> }
> @@ -2902,8 +2912,9 @@ static int bond_ab_arp_inspect(struct bo
> * active. This avoids bouncing, as the last receive
> * times need a full ARP monitor cycle to be updated.
> */
> - if (!time_after_eq(jiffies, slave->jiffies +
> - 2 * delta_in_ticks))
> + if (time_in_range(jiffies,
> + slave->jiffies - delta_in_ticks,
> + slave->jiffies + 2 * delta_in_ticks))
> continue;
>
> /*
> @@ -2921,8 +2932,10 @@ static int bond_ab_arp_inspect(struct bo
> */
> if (slave->state == BOND_STATE_BACKUP &&
> !bond->current_arp_slave &&
> - time_after(jiffies, slave_last_rx(bond, slave) +
> - 3 * delta_in_ticks)) {
> + !time_in_range(jiffies,
> + slave_last_rx(bond, slave) - delta_in_ticks,
> + slave_last_rx(bond, slave) + 3 * delta_in_ticks)) {
> +
> slave->new_link = BOND_LINK_DOWN;
> commit++;
> }
> @@ -2934,10 +2947,13 @@ static int bond_ab_arp_inspect(struct bo
> * the bond has an IP address)
> */
> if ((slave->state == BOND_STATE_ACTIVE) &&
> - (time_after_eq(jiffies, dev_trans_start(slave->dev) +
> - 2 * delta_in_ticks) ||
> - (time_after_eq(jiffies, slave_last_rx(bond, slave)
> - + 2 * delta_in_ticks)))) {
> + (!time_in_range(jiffies,
> + dev_trans_start(slave->dev) - delta_in_ticks,
> + dev_trans_start(slave->dev) + 2 * delta_in_ticks) ||
> + (!time_in_range(jiffies,
> + slave_last_rx(bond, slave) - delta_in_ticks,
> + slave_last_rx(bond, slave) + 2 * delta_in_ticks)))) {
> +
> slave->new_link = BOND_LINK_DOWN;
> commit++;
> }
> @@ -2964,7 +2980,9 @@ static void bond_ab_arp_commit(struct bo
>
> case BOND_LINK_UP:
> if ((!bond->curr_active_slave &&
> - time_before_eq(jiffies,
> + time_in_range(jiffies,
> + dev_trans_start(slave->dev) -
> + delta_in_ticks,
> dev_trans_start(slave->dev) +
> delta_in_ticks)) ||
> bond->curr_active_slave != slave) {
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists