NotificationsYou must be signed in to change notification settings
Fork34k
Star71.3k

gh-78724: raise RuntimeError's when calling methods on non-ready Struct()'s#143643

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

serhiy-storchaka merged 14 commits intopython:mainfromskirpichev:deprecate-struct-init/78724

Jan 11, 2026

Merged

gh-78724: raise RuntimeError's when calling methods on non-ready Struct()'s#143643

serhiy-storchaka merged 14 commits intopython:mainfromskirpichev:deprecate-struct-init/78724

Jan 11, 2026

Conversation

Copy link

Member

skirpichev commentedJan 10, 2026•
edited
Loading

Issue:_struct.Struct: calling functions without calling __init__ results in SystemError #78724

skirpichev added5 commits

January 3, 2026 10:36

pythongh-143379: fix UAF in struct.Struct.pack() when object modified…

a1c2603

… by dunder methods

Merge branch 'master' into fix-uaf-s_pack_internal/143379

716e28c

address review: implement a different solution with a mutex flag

e611953

Now modification of the Struct() while packing trigger a RuntimeError

+ clean the mess

ab62167

pythongh-78724: raise RuntimeError's when calling methods on non-read…

3b18427

…y Struct()'sCalling the ``Struct.__new__()`` dunder without required argument now isdeprecated.  Calling the ``Struct.__init__()`` dunder method oninitialized object now also is deprecated.

bedevere-appbot mentioned this pull request

Jan 10, 2026

_struct.Struct: calling functions without calling __init__ results in SystemError#78724

Closed

skirpichev mentioned this pull request

Jan 10, 2026

gh-143379: fix UAF in struct.Struct.pack() when object modified by dunder methods#143382

Closed

This comment was marked as outdated.

skirpichev marked this pull request as ready for review

January 10, 2026 07:55

bedevere-appbot added the awaiting review label

Jan 10, 2026

skirpichev requested a review fromserhiy-storchaka

January 10, 2026 07:56

serhiy-storchaka reviewed

Jan 10, 2026

View reviewed changes

Copy link

Member

serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

This issue is relatively independed from the concurrent packing issue, so I suggest fir it separately. We should not add deprecation in maintained versions, so I suggest to only leave checks that Struct was initialized, and add the deprecation in a separate PR.

prepare_s() should be rewritten so it only setss_codes and other fields when it is successful.

Modules/_struct.c OutdatedShow resolvedHide resolved

Modules/_struct.c Outdated

		{
		PyObject *self;

		if (PyTuple_GET_SIZE(args) != 1

Copy link

Member

serhiy-storchakaJan 10, 2026

Choose a reason for hiding this comment

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

I do not see how can this be useful.

Copy link

MemberAuthor

skirpichevJan 10, 2026

Choose a reason for hiding this comment

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

This is to trigger deprecation warning in a following scenario:

>>>import struct>>>classMyStruct(struct.Struct):...def__init__(self):...super().__init__('>h')...         >>> my_struct= MyStruct()<python-input-2>:1: DeprecationWarning: Struct().__new__() has one required argument

PS: deprecation part moved to#143659.

Modules/_struct.c OutdatedShow resolvedHide resolved

serhiy-storchaka added needs backport to 3.13

bugs and security fixes

needs backport to 3.14bugs and security fixes labels

Jan 10, 2026

skirpichev marked this pull request as draft

January 10, 2026 10:14

bedevere-appbot removed the awaiting review label

Jan 10, 2026

skirpichev added3 commits

January 10, 2026 13:15

Merge branch 'master' into deprecate-struct-init/78724

54dbea8

revert stuff frompython#143382

b6c8bed

revert deprecations (TBD: a separate pr)

f542173

Copy link

MemberAuthor

skirpichev commentedJan 10, 2026

prepare_s() should be rewritten so it only sets s_codes and other fields when it is successful.

I think it's already the case. We can move a bit s_size/s_len assignments, but as far as we interested only in s_codes - I'm not sure if it's worth. There should be no failures aftercodes = PyMem_Malloc(...).

address review: redo patch to use s_codes

69a5eec

skirpichev marked this pull request as ready for review

January 10, 2026 11:45

bedevere-appbot added the awaiting review label

Jan 10, 2026

skirpichev requested a review fromserhiy-storchaka

January 10, 2026 11:46

Merge branch 'main' into deprecate-struct-init/78724

0e6af95

Copy link

Member

serhiy-storchaka commentedJan 10, 2026

Well, it does not matter. It would reduce the chance of race condition, but nobody should run__init__() andpack() concurrently, there are no reasons to do this, and we should not support this.

serhiy-storchaka reviewed

Jan 10, 2026

View reviewed changes

Lib/test/test_struct.py OutdatedShow resolvedHide resolved

Modules/_struct.c OutdatedShow resolvedHide resolved

address review: -asserts + redo test

8ba6b33

serhiy-storchaka reviewed

Jan 10, 2026

View reviewed changes

Lib/test/test_struct.py OutdatedShow resolvedHide resolved

Lib/test/test_struct.py Outdated

		self.assertRaises(RuntimeError, meth, spam)
		self.assertRaises(RuntimeError, S.iter_unpack, 1)
		self.assertRaises(RuntimeError, S.pack, 1)
		self.assertRaises(RuntimeError, S.pack_into, 1)

Copy link

Member

serhiy-storchakaJan 10, 2026

Choose a reason for hiding this comment

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

S.pack_into(buffer, offset, v1, v2, ...)

Copy link

MemberAuthor

skirpichevJan 10, 2026

Choose a reason for hiding this comment

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

BTW, I think that argument processing in the struct's functions/methods should be refactored and transformed to the AC. E.g. S.pack_info could check required arguments before validation of the soself struct.

Though, this belongs to a separate issue.

Copy link

MemberAuthor

skirpichevJan 11, 2026

Choose a reason for hiding this comment

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

AC cleanup goes to#143673

address review: adjust tests

ea0f8e2

Copy link

MemberAuthor

skirpichev commentedJan 10, 2026

Well, it does not matter. It would reduce the chance of race condition, but nobody should run__init__() andpack() concurrently, there are no reasons to do this, and we should not support this.

I guess this belongs to the#143382 pr thread.

skirpichev requested a review fromserhiy-storchaka

January 11, 2026 11:09

serhiy-storchaka reviewed

Jan 11, 2026

View reviewed changes

Modules/_struct.cShow resolvedHide resolved

Modules/_struct.c OutdatedShow resolvedHide resolved

skirpichev added2 commits

January 11, 2026 18:21

Merge branch 'master' into deprecate-struct-init/78724

c71be45

address review: revert unrelated change

b61bddb

serhiy-storchaka approved these changes

Jan 11, 2026

View reviewed changes

Copy link

Member

serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

bedevere-appbot added awaiting merge and removed awaiting review labels

Jan 11, 2026

serhiy-storchakaenabled auto-merge (squash)

January 11, 2026 15:47

serhiy-storchaka merged commit515ae40 intopython:main

Jan 11, 2026

50 checks passed

bedevere-appbot removed the awaiting merge label

Jan 11, 2026

Copy link

miss-islington-appbot commentedJan 11, 2026

Thanks@skirpichev for the PR, and@serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Jan 11, 2026

pythongh-78724: Raise RuntimeError's when calling methods on non-read…

660b508

…y Struct()'s (pythonGH-143643)(cherry picked from commit515ae40)Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

Copy link

miss-islington-appbot commentedJan 11, 2026

Sorry,@skirpichev and@serhiy-storchaka, I could not cleanly backport this to3.13 due to a conflict.
Please backport usingcherry_picker on command line.

cherry_picker 515ae4078dffa0b74e5e5431462c2f4fe4563ffa 3.13

miss-islington-appbot assignedserhiy-storchaka

Jan 11, 2026

Copy link

bedevere-appbot commentedJan 11, 2026

GH-143695 is a backport of this pull request to the3.14 branch.

bedevere-appbot removed the needs backport to 3.14bugs and security fixes label

Jan 11, 2026

skirpichev deleted the deprecate-struct-init/78724 branch

January 11, 2026 18:04

skirpichev self-assigned this

Jan 11, 2026

Copy link

bedevere-appbot commentedJan 11, 2026

GH-143714 is a backport of this pull request to the3.13 branch.

bedevere-appbot removed the needs backport to 3.13bugs and security fixes label

Jan 11, 2026

skirpichev removed their assignment

Jan 11, 2026

serhiy-storchaka pushed a commit that referenced this pull request

Jan 12, 2026

[3.14]gh-78724: Raise RuntimeError's when calling methods on non-rea…

8f85adc

…dy Struct()'s (GH-143643) (GH-143695)(cherry picked from commit515ae40)Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

serhiy-storchaka pushed a commit that referenced this pull request

Jan 12, 2026

[3.13]gh-78724: Raise RuntimeError's when calling methods on non-rea…

8fd4435

…dy Struct()'s (GH-143643) (GH-143714)(cherry picked from commit515ae40)

reidenong pushed a commit to reidenong/cpython that referenced this pull request

Jan 12, 2026

pythongh-78724: Raise RuntimeError's when calling methods on non-read…

e3ef2fc

…y Struct()'s (pythonGH-143643)

Labels

None yet

Movatterモバイル変換

Uh oh!

gh-78724: raise RuntimeError's when calling methods on non-ready Struct()'s#143643

gh-78724: raise RuntimeError's when calling methods on non-ready Struct()'s#143643

Uh oh!

Conversation

skirpichev commentedJan 10, 2026• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

serhiy-storchakaJan 10, 2026

Choose a reason for hiding this comment

Uh oh!

skirpichevJan 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

skirpichev commentedJan 10, 2026

Uh oh!

serhiy-storchaka commentedJan 10, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

serhiy-storchakaJan 10, 2026

Choose a reason for hiding this comment

Uh oh!

skirpichevJan 10, 2026

Choose a reason for hiding this comment

Uh oh!

skirpichevJan 11, 2026

Choose a reason for hiding this comment

Uh oh!

skirpichev commentedJan 10, 2026

Uh oh!

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

miss-islington-appbot commentedJan 11, 2026

Uh oh!

miss-islington-appbot commentedJan 11, 2026

Uh oh!

bedevere-appbot commentedJan 11, 2026

Uh oh!

bedevere-appbot commentedJan 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

skirpichev commentedJan 10, 2026•
edited
Loading