Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't decref PyBytesObject before formatting an error with it #1299

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

jorio
Copy link
Contributor

@jorio jorio commented Jun 2, 2024

Some C functions call Py_XDECREF on a PyBytesObject (representing a string), and then they pass a pointer within that object to Error_set_str. This causes the error message to appear corrupted if garbage collection occurs at just the right time.

For example, init_file_backend does something like this:

PyBytesObject *py_path = ...;

const char* path = PyBytes_AS_STRING(py_path);

err = git_repository_open_ext(&repository, path, flags, NULL);

Py_XDECREF(py_path);  // <--- too early!

if (err < 0) {
    Error_set_str(err, path);  // <--- path may be gone due to XDECREF above
    goto cleanup;
}

The issue is that PyBytes_AsString returns a pointer within py_path – it doesn't copy the string. So, Py_XDECREF may cause py_path to be collected at any time, possibly garbling the value of path.

When my application invokes Repository.__init__ with a path that does not exist, such as "/tmp/submoroot/submosub", then pygit2 may ultimately raise this:

repository.py:1607, in __init__
    path_backend = init_file_backend(path, int(flags))
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
_pygit2.GitError: Repository not found at dtmp/subdtmp/subdtmp/subdtmp/su

Note that the path in the error message (dtmp/subdtmp/subdtmp/subdtmp/su) is garbled.

This PR fixes this by calling Py_XDECREF(py_path) after the error message is built. I'm not sure how to distill this into a unit test because the GC needs to kick in at just the right time to exhibit the faulty behavior.

@jdavid jdavid merged commit 7bea492 into libgit2:master Jun 3, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants