On 22 September 2017 at 14:24, Shashank Ram <r...@vmware.com> wrote:
> Conntrack, Conntrack-related, Stt, and IP fragmentation
> have cleaner threads that run periodically to clean
> up their respective tables. During driver unload,
> OvsExtDetach() calls into routines that are meant
> for explicitly cleaning these tables up and freeing
> the resources associated with these threads.
>
> If during driver unload, these cleaner threads run
> immediately after the resources are freed, such as locks
> used by these threads, then the cleaner threads result
> in a kernel crash since they try to acquire locks
> that have already been freed.
>
> For eg, OvsIpFragmentEntryCleaner() caused a kernel
> crash because it tried to acquire a lock that was
> already freed by OvsCleanupIpFragment().
>
> The fix is to simply exit the cleaner thread if the
> lock associated with the thread is not initialized,
> because the only way the threads can run when the lock
> is invalid is when the lock has been freed up during
> driver unload.
>
> Testint done:
> Verified that cleaner threads run as expected without
> crashing during driver unload.
>
> Signed-off-by: Shashank Ram <r...@vmware.com>
>
Applied to master and 2.8. Thanks!
> ---
> datapath-windows/ovsext/Conntrack-related.c | 6 +++++-
> datapath-windows/ovsext/Conntrack.c | 6 +++++-
> datapath-windows/ovsext/IpFragment.c | 6 +++++-
> datapath-windows/ovsext/Stt.c | 4 ++++
> 4 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Conntrack-related.c
> b/datapath-windows/ovsext/Conntrack-related.c
> index 16ed6f7..ec4b536 100644
> --- a/datapath-windows/ovsext/Conntrack-related.c
> +++ b/datapath-windows/ovsext/Conntrack-related.c
> @@ -181,10 +181,14 @@ OvsCtRelatedEntryCleaner(PVOID data)
> POVS_CT_THREAD_CTX context = (POVS_CT_THREAD_CTX)data;
> PLIST_ENTRY link, next;
> POVS_CT_REL_ENTRY entry;
> + LOCK_STATE_EX lockState;
> BOOLEAN success = TRUE;
>
> while (success) {
> - LOCK_STATE_EX lockState;
> + if (ovsCtRelatedLockObj == NULL) {
> + /* Lock has been freed by 'OvsCleanupCtRelated()' */
> + break;
> + }
> NdisAcquireRWLockWrite(ovsCtRelatedLockObj, &lockState, 0);
> if (context->exit) {
> NdisReleaseRWLock(ovsCtRelatedLockObj, &lockState);
> diff --git a/datapath-windows/ovsext/Conntrack.c
> b/datapath-windows/ovsext/Conntrack.c
> index f0d135b..3203411 100644
> --- a/datapath-windows/ovsext/Conntrack.c
> +++ b/datapath-windows/ovsext/Conntrack.c
> @@ -980,10 +980,14 @@ OvsConntrackEntryCleaner(PVOID data)
> POVS_CT_THREAD_CTX context = (POVS_CT_THREAD_CTX)data;
> PLIST_ENTRY link, next;
> POVS_CT_ENTRY entry;
> + LOCK_STATE_EX lockState;
> BOOLEAN success = TRUE;
>
> while (success) {
> - LOCK_STATE_EX lockState;
> + if (ovsConntrackLockObj == NULL) {
> + /* Lock has been freed by 'OvsCleanupConntrack()' */
> + break;
> + }
> NdisAcquireRWLockWrite(ovsConntrackLockObj, &lockState, 0);
> if (context->exit) {
> NdisReleaseRWLock(ovsConntrackLockObj, &lockState);
> diff --git a/datapath-windows/ovsext/IpFragment.c
> b/datapath-windows/ovsext/IpFragment.c
> index fee361a..ad48834 100644
> --- a/datapath-windows/ovsext/IpFragment.c
> +++ b/datapath-windows/ovsext/IpFragment.c
> @@ -444,10 +444,14 @@ OvsIpFragmentEntryCleaner(PVOID data)
> POVS_IPFRAG_THREAD_CTX context = (POVS_IPFRAG_THREAD_CTX)data;
> PLIST_ENTRY link, next;
> POVS_IPFRAG_ENTRY entry;
> + LOCK_STATE_EX lockState;
> BOOLEAN success = TRUE;
>
> while (success) {
> - LOCK_STATE_EX lockState;
> + if (ovsIpFragmentHashLockObj == NULL) {
> + /* Lock has been freed by 'OvsCleanupIpFragment()' */
> + break;
> + }
> NdisAcquireRWLockWrite(ovsIpFragmentHashLockObj, &lockState, 0);
> if (context->exit) {
> NdisReleaseRWLock(ovsIpFragmentHashLockObj, &lockState);
> diff --git a/datapath-windows/ovsext/Stt.c b/datapath-windows/ovsext/Stt.c
> index b6236fd..f98070f 100644
> --- a/datapath-windows/ovsext/Stt.c
> +++ b/datapath-windows/ovsext/Stt.c
> @@ -551,6 +551,10 @@ OvsSttDefragCleaner(PVOID data)
> BOOLEAN success = TRUE;
>
> while (success) {
> + if (&OvsSttSpinLock == NULL) {
> + /* Lock has been freed by 'OvsCleanupSttDefragmentation()' */
> + break;
> + }
> NdisAcquireSpinLock(&OvsSttSpinLock);
> if (context->exit) {
> NdisReleaseSpinLock(&OvsSttSpinLock);
> --
> 2.9.3.windows.2
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
