Descriptor resolution should own arguments passed to descriptors

[tekknolagi]

BPO	46561
Nosy	@rhettinger, @markshannon, @tekknolagi, @sweeneyde
PRs	bpo-46561: Ensure operands to __get__ survive the call #30979

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2022-01-28.07:06:14.782>
labels = ['expert-C-API', '3.8', '3.9', '3.10', '3.11']
title = 'Descriptor resolution should own arguments passed to descriptors'
updated_at = <Date 2022-02-01.15:21:47.460>
user = 'https://github.com/tekknolagi'

bugs.python.org fields:

activity = <Date 2022-02-01.15:21:47.460>
actor = 'rhettinger'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['C API']
creation = <Date 2022-01-28.07:06:14.782>
creator = 'tekknolagi'
dependencies = []
files = []
hgrepos = []
issue_num = 46561
keywords = ['patch']
message_count = 7.0
messages = ['411978', '411979', '412035', '412040', '412049', '412250', '412277']
nosy_count = 4.0
nosy_names = ['rhettinger', 'Mark.Shannon', 'tekknolagi', 'Dennis Sweeney']
pr_nums = ['30979']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue46561'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10', 'Python 3.11']

[tekknolagi]

Currently the descriptor (self) argument to __get__ is passed borrowed, since _PyType_LookupId returns a borrowed reference (see _PyObject_LookupSpecial and lookup_maybe_method in Objects/typeobject.c). This should instead own the reference.

[sweeneyde]

Why? Callee-borrowing-from-caller is the established norm across the C API. You mention use-after-free, but can you elaborate on how that can happen in practice?

https://docs.python.org/3/extending/extending.html?highlight=borrowed#ownership-rules says:

"""When you pass an object reference into another function, in general, the function borrows the reference from you — if it needs to store it, it will use Py_INCREF() to become an independent owner."""

[tekknolagi]

Hi Dennis,

Sorry, let me be more clear. CPython in general ensures that objects passed in as arguments to a function will live for the duration of the function call if they are otherwise untouched. As it is now, this invariant is not maintained when calling the __get__ descriptor. Right now, it is not only borrowed by the callee but also not owned by the caller (!).

Max

[tekknolagi]

Ah, and another piece of the puzzle: this can happen in runtimes like Cinder that provide their own native code entrypoints to functions like a __get__.

[sweeneyde]

Hi Max,

My apologies -- my first message was probably too dismissive.

Is there any way to make the existing code crash with pure Python, and can we then add such a test case? If not with pure Python, then perhaps with some minimal reproducible example using the C API?

Also, does the change have any effect on microbenchmarks involving the modified functions?