Skip to content

GH-49917: [Python] Remove Py_XDECREF to avoid Use-After-Free on PyList_SetItem in SparseCSFTensorToNdarray - #49916

Merged
raulcd merged 2 commits into
apache:mainfrom
wr-web:main
May 7, 2026
Merged

GH-49917: [Python] Remove Py_XDECREF to avoid Use-After-Free on PyList_SetItem in SparseCSFTensorToNdarray#49916
raulcd merged 2 commits into
apache:mainfrom
wr-web:main

Conversation

@wr-web

@wr-webwr-web commented May 5, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

Py_DECREF(item) in PyList_SetItem will cause Use-After-Free bug if PyList_SetItem(indptr.obj(), i, item) < 0 is true, cause PyList_SetItem always steals a reference to the item, even when it fails.

What changes are included in this PR?

  1. Remove Py_DECREF(item) in PyList_SetItem error path.

Are these changes tested?

By CI.

Are there any user-facing changes?

No.

wr-web added 2 commits May 5, 2026 13:05
Remove incorrect reference count decrement for item.
apache#49915
Remove unnecessary reference count decrement for item.
apache#49915
@wr-web
wr-web requested review from AlenkaF, raulcd and rok as code ownersMay 5, 2026 05:07
@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

@raulcdraulcd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Can you use the PR description template instead of removing it?
Can you also use the expected title for the PR as described on the automated message?

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels May 6, 2026
@pitrou

Copy link
Copy Markdown
Member

(PR contents look good otherwise: PyList_SetItem always steals a reference to the item, even when it fails)

@wr-webwr-web changed the title Fix memory management in numpy_convert.cc[Python] GH-49915: [numpy_convert] memory management(Use-After-Free) Bugs for PyList_SetItem in SparseCSFTensorToNdarrayMay 6, 2026
@wr-web

Copy link
Copy Markdown
ContributorAuthor

Thanks for the PR. Can you use the PR description template instead of removing it? Can you also use the expected title for the PR as described on the automated message?

Done

@raulcdraulcd changed the title [Python] GH-49915: [numpy_convert] memory management(Use-After-Free) Bugs for PyList_SetItem in SparseCSFTensorToNdarrayGH-49915: [Python] numpy_convert.cc memory management(Use-After-Free) Bugs for PyList_SetItem in SparseCSFTensorToNdarrayMay 6, 2026
rok
rok approved these changes May 6, 2026
@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@raulcdraulcd changed the title GH-49915: [Python] numpy_convert.cc memory management(Use-After-Free) Bugs for PyList_SetItem in SparseCSFTensorToNdarrayGH-49917: [Python] numpy_convert.cc memory management(Use-After-Free) Bugs for PyList_SetItem in SparseCSFTensorToNdarrayMay 6, 2026
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #49917has been automatically assigned in GitHub to PR creator.

@github-actionsgithub-actionsBot added awaiting merge Awaiting merge and removed awaiting changes Awaiting changes labels May 6, 2026

@raulcdraulcd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for the PR!
I've done a minor update to the title so our automated tools don't trip.

@raulcdraulcd changed the title GH-49917: [Python] numpy_convert.cc memory management(Use-After-Free) Bugs for PyList_SetItem in SparseCSFTensorToNdarrayGH-49917: [Python] Remove Py_XDECREF to avoid Use-After-Free on PyList_SetItem in SparseCSFTensorToNdarrayMay 7, 2026
@raulcd
raulcd merged commit 15c2a3b into apache:mainMay 7, 2026
26 of 30 checks passed
@raulcdraulcd removed the awaiting merge Awaiting merge label May 7, 2026
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 0 benchmarking runs that have been run so far on merge-commit 15c2a3b.

None of the specified runs were found on the Conbench server.

The full Conbench report has more details.

@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 15c2a3b.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them.

Mottl pushed a commit to Mottl/arrow that referenced this pull request May 26, 2026
… `PyList_SetItem` in `SparseCSFTensorToNdarray` (apache#49916)
### Rationale for this change
Py_DECREF(item) in PyList_SetItem will cause Use-After-Free bug if `PyList_SetItem(indptr.obj(), i, item) < 0` is `true`, cause `PyList_SetItem` always steals a reference to the item, even when it fails.
### What changes are included in this PR?
1. Remove Py_DECREF(item) in PyList_SetItem error path.
### Are these changes tested?
By CI.
### Are there any user-facing changes?
No.
* GitHub Issue: apache#49917
Authored-by: Wang Rui <55612496+wr-web@users.noreply.github.com>
Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@wr-web@pitrou@rok@raulcd