Remove incorrect reference count decrement for dim.#1022
Open
wr-web wants to merge 1 commit intoDeepRec-AI:mainfrom
Open
Remove incorrect reference count decrement for dim.#1022wr-web wants to merge 1 commit intoDeepRec-AI:mainfrom
wr-web wants to merge 1 commit intoDeepRec-AI:mainfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue
#1021
Description
A reference to item is incorrectly decremented after a failed PyTuple_SetItem call, leading to a potential use-after-free vulnerability or double-decrement crash.
Affected Code
DeepRec/tensorflow/python/eager/pywrap_tensor.cc
Lines 593 to 598 in d1c5a6e
Root Cause
PyTuple_SetItem Source Code
PyTuple_SetItem Document
PyList_SetItem Python Forum Discussion
Therefore, whether the function succeeds or fails, it will steal a reference count of the third argument. It must not call
Py_XDECREF/Py_DECREFin case of failure.Evidence:Reference Counting and Ownership in CPython Native API
Borrowed Reference
A borrowed reference is a reference obtained from an object that you don't own. You don't need to decrement its reference count when you're done with it, but you must ensure the object stays alive while you're using it (e.g., by creating an owned reference with
Py_INCREFif necessary). Borrowed references are typically returned by functions likePyList_GetItem(), which returns an item from a list without incrementing its reference count.New Reference
A new reference (also called an "owned reference") is a reference that you have ownership of. When you receive a new reference from a function (such as
PyObject_New()orPy_BuildValue()), you are responsible for callingPy_DECREF()on it when you no longer need it to properly decrement its reference count. Failure to do so causes memory leaks.Stolen Reference (Stealing)
A stolen reference is when a function takes ownership of a reference you pass to it. When you pass an object reference to a function that "steals" it, you no longer own that reference, and you should not call
Py_DECREF()on it afterward. The function assumes full responsibility for managing the reference count of that object.Example:
PyTuple_SetItemPyTuple_SetItemis a classic example of a reference-stealing function. According to the documentation:However, a critical ambiguity in the documentation is that it does not clearly state whether the reference is stolen in the case of function failure. This is fundamentally an all-or-nothing problem: when you pass an object to a stealing function, you must understand whether ownership is unconditionally transferred or only transferred on success.
Looking at the CPython source code clarifies this behavior:
As demonstrated in the source code above,
PyTuple_SetItemunconditionally callsPy_XDECREF(newitem)when the type check fails—meaning it always steals the reference, even on failure. The function takes ownership ofnewitemregardless of whether it successfully inserts the item into the tuple.This behavior has serious implications for correct API usage. Consider the following incorrect code:
This code is defective because it leads to a use-after-free vulnerability. Since
PyTuple_SetItemalready stole the reference (and decremented it on failure viaPy_XDECREF), the additionalPy_DECREF(something)in the error-handling block causes a double decrement, potentially leading to an assertion failure in debug builds or memory corruption and crashes in release builds.The correct pattern is simply:
In summary, when dealing with stealing functions in the CPython API, you must relinquish all ownership responsibility for the passed reference and never decrement it after the call, regardless of the return value. Always consult the source code or thoroughly documented behavior to confirm whether a function truly provides an all-or-nothing stealing guarantee.