Fix use after free (#4559)

zou3519 · soumith · commit f98c795b71d6 · 2018-02-06T21:44:32.000-08:00
In `THPTensor_(_convertToTensorIndexers)`, a `vector&lt;THPIndexTensor&gt;` is
created by constructing `THPTensor`s from sequences/tensors/etc. Each
`THPIndexTensor` is then freed with the following:

```
for (auto&amp; idx : indexers) {
  THIndexTensor_(free)(LIBRARY_STATE idx-&gt;cdata);
  Py_DECREF(idx);
}
```

This is a problem because `Py_DECREF(idx)` will turn `idx-&gt;ob_refcnt` to 0 since this function
created the relevant `THPIndexTensor`s and owns them, causing `THPTensor_(dealloc)` to be
called. `THPTensor_(dealloc)` already has a line that calls
`THIndexTensor_(free)(LIBRARY_STATE idx-&gt;cdata)`.

So `THIndexTensor_(free)(LIBRARY_STATE idx-&gt;cdata)` gets called twice on the same
`cdata`. After the first call frees `cdata`, the second attempts to access flags/members of `cdata` to
determine if it should free it.
diff --git a/torch/csrc/generic/Tensor.cpp b/torch/csrc/generic/Tensor.cpp
@@ -829,7 +829,6 @@ static bool THPTensor_(_convertToTensorIndexers)(
 
           // Clean up Indexers
           for (auto& idx : indexers) {
-            THIndexTensor_(free)(LIBRARY_STATE idx->cdata);
             Py_DECREF(idx);
           }
           return false;
@@ -890,7 +889,6 @@ static bool THPTensor_(_convertToTensorIndexers)(
 
           // Clean up Indexers
           for (auto& idx : indexers) {
-            THIndexTensor_(free)(LIBRARY_STATE idx->cdata);
             Py_DECREF(idx);
           }
 
@@ -909,15 +907,13 @@ static bool THPTensor_(_convertToTensorIndexers)(
 
     // Clean up Indexers
     for (auto& idx : indexers) {
-      THIndexTensor_(free)(LIBRARY_STATE idx->cdata);
       Py_DECREF(idx);
     }
     return false;
   }
 
   // Clean up Indexers
   for (auto& idx : indexers) {
-    THIndexTensor_(free)(LIBRARY_STATE idx->cdata);
     Py_DECREF(idx);
   }
   return true;

Original file line number	Diff line number	Diff line change
`@@ -829,7 +829,6 @@ static bool THPTensor_(_convertToTensorIndexers)(`
`829`	`829`
`830`	`830`	`// Clean up Indexers`
`831`	`831`	`for (auto& idx : indexers) {`
`832`		`- THIndexTensor_(free)(LIBRARY_STATE idx->cdata);`
`833`	`832`	`Py_DECREF(idx);`
`834`	`833`	`}`
`835`	`834`	`return false;`
`@@ -890,7 +889,6 @@ static bool THPTensor_(_convertToTensorIndexers)(`
`890`	`889`
`891`	`890`	`// Clean up Indexers`
`892`	`891`	`for (auto& idx : indexers) {`
`893`		`- THIndexTensor_(free)(LIBRARY_STATE idx->cdata);`
`894`	`892`	`Py_DECREF(idx);`
`895`	`893`	`}`
`896`	`894`
`@@ -909,15 +907,13 @@ static bool THPTensor_(_convertToTensorIndexers)(`
`909`	`907`
`910`	`908`	`// Clean up Indexers`
`911`	`909`	`for (auto& idx : indexers) {`
`912`		`- THIndexTensor_(free)(LIBRARY_STATE idx->cdata);`
`913`	`910`	`Py_DECREF(idx);`
`914`	`911`	`}`
`915`	`912`	`return false;`
`916`	`913`	`}`
`917`	`914`
`918`	`915`	`// Clean up Indexers`
`919`	`916`	`for (auto& idx : indexers) {`
`920`		`- THIndexTensor_(free)(LIBRARY_STATE idx->cdata);`
`921`	`917`	`Py_DECREF(idx);`
`922`	`918`	`}`
`923`	`919`	`return true;`