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
| ||
|
Date: Mon, 20 Dec 2010 13:11:46 -0800 (PST) From: David Miller <davem@...emloft.net> To: tj@...nel.org Cc: mchan@...adcom.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH net-next-2.6] bnx2: remove cancel_work_sync() from remove_one From: Tejun Heo <tj@...nel.org> Date: Wed, 15 Dec 2010 14:52:29 +0100 > After looking through the code, I don't think this is necessarily > correct. ->ndo_close() doesn't guarantee that the watchdog timer has > finished running (the timer is deleted with del_timer() not > del_timer_sync()). ie. the watchdog timer could still be running > after ->ndo_close() and may schedule reset_task. If remove_one > doesn't flush the task, it may still be running when remove_one() is > called. > > David, am I missing something? Wouldn't it cleaner to guarantee that > ->ndo_close() is called with the guarantee that the watchdog timer is > not running anymore? It would but we can't just make the change over to del_timer_sync() otherwise we'd deadlock on netif_tx_lock(). But I think things might be OK as-is. The timer is deleted by dev_deactivate_many() which resets the qdisc to the no-op qdisc. Then it deletes the timer. Any running timer will complete or see the no-op qdisc attached and return immediately. synchronize_rcu() is then executed which guarentees completion. Since both the watchdog timer itself and the del_timer() call run with netif_tx_lock() held, this makes sure the timer, once deleted, will only see the no-op qdisc and return immediately if it is amidst running, else it has already returned when the timer delete completes. So we might be OK here. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists