Uh oh!
There was an error while loading. Please reload this page.
gh-89928: Fix integer conversion of device numbers. - #31794
Merged
Conversation
Fix os.major(), os.minor() and os.makedev(). Support device numbers larger than 2**63-1. Support non-existent device number (NODEV).
bedevere-bot
commented
Mar 10, 2022
🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit c2d082b 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
ambv
commented
May 17, 2022
Contributor
This missed the boat for inclusion in Python 3.9 which accepts security fixes only as of today. |
iritkatriel
previously requested changes
Nov 27, 2022
iritkatriel
left a comment
Member
There was a problem hiding this comment.
This has merge conflicts now.
bedevere-bot
commented
Nov 27, 2022
When you're done making the requested changes, leave the comment: |
serhiy-storchaka
commented
Dec 17, 2023
MemberAuthor
!buildbot freebsd |
bedevere-bot
commented
Dec 17, 2023
🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit 17c02dc 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
serhiy-storchaka
commented
Dec 17, 2023
MemberAuthor
I have made the requested changes; please review again. |
bedevere-bot
commented
Jun 3, 2024
🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit 97e71d3 🤖 If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13. |
miss-islington pushed a commit
to miss-islington/cpython
that referenced
this pull request
Jun 4, 2024
) Fix os.major(), os.minor() and os.makedev(). Support device numbers larger than 2**63-1. Support non-existent device number (NODEV). (cherry picked from commit 7111d96) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Sorry, @serhiy-storchaka, I could not cleanly backport this to |
GH-120053 is a backport of this pull request to the 3.13 branch. |
serhiy-storchaka added a commit
to serhiy-storchaka/cpython
that referenced
this pull request
Jun 4, 2024
…onGH-31794) Fix os.major(), os.minor() and os.makedev(). Support device numbers larger than 2**63-1. Support non-existent device number (NODEV). (cherry picked from commit 7111d96) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
GH-120054 is a backport of this pull request to the 3.12 branch. |
vstinner
commented
Jun 4, 2024
Member
Thanks for the fix @serhiy-storchaka! |
serhiy-storchaka added a commit
that referenced
this pull request
Jun 4, 2024
rlbdv added a commit
to rlbdv-ci/bup
that referenced
this pull request
May 25, 2026
Tests on freebsd with zfs failed with OverflowErrors because zfs was creating a device (dev_t) with the high bit set (which is is known to to), and it turns out that python hasn't handled that consistently. Recent python versions handle it as unsigned, ignoring the platform's definition (e.g. on macos it's signed), in nearly all cases. Though it looks like os.makedev() is still broken. Python improved the "high bit set" dev_t handling here (switching, as mentioned, by trying to shift to treat them as unsigned): [1] 7111d9605f9db7aa0b095bb8ece7ccc0b8115c3f gh-89928: Fix integer conversion of device numbers (GH-31794) python/cpython#31794 And those changes have been backported to an increasing number of python minor releases. I believe we were exposed to these errors when I removed our custom stat function: commit ae032d6 Drop our custom stat functions in favor of python's To date, bup has also required dev_t (and some other stat fields) to be signed and no more than eight bytes via use of Q in the INDEX_SIG pack format for index entries. Whether or not python also attempts to treat all of these consistently as unsigned has not been investigated. To address the dev_t issue, and to make sure we handle all the other stat fields consistently over time and across python versions, bring back and enhance our custom stat functions, provide compatible versions of major() and minor(), and wrap os.mknod() to make sure it only sees the unsigned device values it expects (we don't just wrap mknod too because it's a bit more complex, and it doesn't appear to have changed behavior over the perids we investigated). Have all of our stat operations work with the actual platform ranges (i.e. preserve unsigned/signed values), and make sure that the index doesn't lose the stat field sign information by providing the platform specific entry type signature to the index from _helpers. So that we don't mishandle the index (even across hosts, say via moving a bup repo from one machine to another), add the entry signature to the index's header and require reindexing if the current platform's entry signature doesn't match the one in the index. Continue to support the previous index format (trivially) so that if the entry signature hasn't actually changed, you don't have to reindex just for the new format. Rename INDEX_SIG to ENTRY_SIG since that's what it is, and rename INDEX_HDR to INDEX_SIG since that's what *it* is. (Given these changes, we could also make the index signature even more dynamic, i.e. shrink all the entries to the actual platform sizes, e.g. on linux, uid and gid could be 4 bytes.) Thanks to Johannes Berg for a good bit of help figuring all this out. Signed-off-by: Rob Browning <rlb@defaultvalue.org> Tested-by: Rob Browning <rlb@defaultvalue.org>
rlbdv added a commit
to rlbdv-ci/bup
that referenced
this pull request
May 25, 2026
Tests on freebsd with zfs failed with OverflowErrors because zfs was creating a device (dev_t) with the high bit set (which is is known to to), and it turns out that python hasn't handled that consistently. Recent python versions handle it as unsigned, ignoring the platform's definition in nearly all cases (e.g. on macos it's signed). Though it looks like os.makedev() is still broken. Python improved the "high bit set" dev_t handling in this commit (switching, as mentioned, to try to treat them as unsigned): [1] 7111d9605f9db7aa0b095bb8ece7ccc0b8115c3f gh-89928: Fix integer conversion of device numbers (GH-31794) python/cpython#31794 And those changes have been backported to an increasing number of python minor releases. (Whether or not python attempts to treat stat fields other than dev_t as unsigned consistently has not been investigated.) I believe we were exposed to these errors when I removed our custom stat function in main: commit ae032d6 Drop our custom stat functions in favor of python's To date, bup has required dev_t (and some other stat fields) to be unsigned too, and no more than eight bytes, via use of Q in the INDEX_SIG pack format for index entries. To address the dev_t issue, and to make sure we handle all the other stat fields consistently over time and across python versions, bring back and enhance our custom stat functions, provide compatible versions of major() and minor(), and wrap python's os.mknod() to make sure it only sees the unsigned device values it expects (we don't wrap mknod too because it's a bit more complex, and it doesn't appear to have changed behavior over the periods we investigated). Have all of our stat operations work with the actual platform ranges (i.e. preserve unsigned/signed values), and make sure that the index doesn't lose the stat field sign information by providing the platform specific entry type signature to the index from _helpers. Finally, so that we don't mishandle the index (even across hosts, say via moving a bup repo from one machine to another), add the entry signature to the index's header and require reindexing if the current platform's signature doesn't match the one in the index. Continue to support the previous index format (trivially) so that if the entry signature hasn't actually changed, you don't have to reindex just for the new format. Rename INDEX_SIG to ENTRY_SIG since that's what it is, and rename INDEX_HDR to INDEX_SIG since that's what *it* is. Thanks to Johannes Berg for a good bit of help figuring all this out. Signed-off-by: Rob Browning <rlb@defaultvalue.org> Tested-by: Rob Browning <rlb@defaultvalue.org>
rlbdv added a commit
to rlbdv-ci/bup
that referenced
this pull request
May 25, 2026
Tests on freebsd with zfs failed with OverflowErrors because zfs was creating a device (dev_t) with the high bit set (which is is known to to), and it turns out that python hasn't handled that consistently. Recent python versions handle it as unsigned, ignoring the platform's definition in nearly all cases (e.g. on macos it's signed). Though it looks like os.makedev() is still broken. Python improved the "high bit set" dev_t handling in this commit (switching, as mentioned, to try to treat them as unsigned): [1] 7111d9605f9db7aa0b095bb8ece7ccc0b8115c3f gh-89928: Fix integer conversion of device numbers (GH-31794) python/cpython#31794 And those changes have been backported to an increasing number of python minor releases. (Whether or not python attempts to treat stat fields other than dev_t as unsigned consistently has not been investigated.) I believe we were exposed to these errors when I removed our custom stat function in main: commit ae032d6 Drop our custom stat functions in favor of python's To date, bup has required dev_t (and some other stat fields) to be unsigned too, and no more than eight bytes, via use of Q in the INDEX_SIG pack format for index entries. To address the dev_t issue, and to make sure we handle all the other stat fields consistently over time and across python versions, bring back and enhance our custom stat functions, provide compatible versions of major() and minor(), and wrap python's os.mknod() to make sure it only sees the unsigned device values it expects (we don't wrap mknod too because it's a bit more complex, and it doesn't appear to have changed behavior over the periods we investigated). Have all of our stat operations work with the actual platform ranges (i.e. preserve unsigned/signed values), and make sure that the index doesn't lose the stat field sign information by providing the platform specific entry type signature to the index from _helpers. Finally, so that we don't mishandle the index (even across hosts, say via moving a bup repo from one machine to another), add the entry signature to the index's header and require reindexing if the current platform's signature doesn't match the one in the index. Continue to support the previous index format (trivially) so that if the entry signature hasn't actually changed, you don't have to reindex just for the new format. Rename INDEX_SIG to ENTRY_SIG since that's what it is, and rename INDEX_HDR to INDEX_SIG since that's what *it* is. Thanks to Johannes Berg for a good bit of help figuring all this out. Signed-off-by: Rob Browning <rlb@defaultvalue.org> Tested-by: Rob Browning <rlb@defaultvalue.org>
rlbdv added a commit
to rlbdv-ci/bup
that referenced
this pull request
May 25, 2026
Tests on freebsd with zfs failed with OverflowErrors because zfs was creating a device (dev_t) with the high bit set (which is is known to to), and it turns out that python hasn't handled that consistently. Recent python versions handle it as unsigned, ignoring the platform's definition in nearly all cases (e.g. on macos it's signed). Though it looks like os.makedev() is still broken. Python improved the "high bit set" dev_t handling in this commit (switching, as mentioned, to try to treat them as unsigned): [1] 7111d9605f9db7aa0b095bb8ece7ccc0b8115c3f gh-89928: Fix integer conversion of device numbers (GH-31794) python/cpython#31794 And those changes have been backported to an increasing number of python minor releases. (Whether or not python attempts to treat stat fields other than dev_t as unsigned consistently has not been investigated.) I believe we were exposed to these errors when I removed our custom stat function in main: commit ae032d6 Drop our custom stat functions in favor of python's To date, bup has required dev_t (and some other stat fields) to be unsigned too, and no more than eight bytes, via use of Q in the INDEX_SIG pack format for index entries. To address the dev_t issue, and to make sure we handle all the other stat fields consistently over time and across python versions, bring back and enhance our custom stat functions, provide compatible versions of major() and minor(), and wrap python's os.mknod() to make sure it only sees the unsigned device values it expects (we don't wrap mknod too because it's a bit more complex, and it doesn't appear to have changed behavior over the periods we investigated). Have all of our stat operations work with the actual platform ranges (i.e. preserve unsigned/signed values), and make sure that the index doesn't lose the stat field sign information by providing the platform specific entry type signature to the index from _helpers. Finally, so that we don't mishandle the index (even across hosts, say via moving a bup repo from one machine to another), add the entry signature to the index's header and require reindexing if the current platform's signature doesn't match the one in the index. Continue to support the previous index format (trivially) so that if the entry signature hasn't actually changed, you don't have to reindex just for the new format. Rename INDEX_SIG to ENTRY_SIG since that's what it is, and rename INDEX_HDR to INDEX_SIG since that's what *it* is. Thanks to Johannes Berg for a good bit of help figuring all this out. Signed-off-by: Rob Browning <rlb@defaultvalue.org> Tested-by: Rob Browning <rlb@defaultvalue.org>
rlbdv added a commit
to rlbdv-ci/bup
that referenced
this pull request
Jun 15, 2026
Tests on freebsd with zfs failed with OverflowErrors because zfs was creating a device (dev_t) with the high bit set (which it is known to), and it turns out that python hasn't handled that consistently. Recent python versions handle it as unsigned, ignoring the platform's definition in nearly all cases (e.g. on macos it's signed). Though it looks like os.makedev() is still broken. Python improved the "high bit set" dev_t handling in this commit (switching, as mentioned, to try to treat them as unsigned): 7111d9605f9db7aa0b095bb8ece7ccc0b8115c3f gh-89928: Fix integer conversion of device numbers (GH-31794) python/cpython#31794 And those changes have been backported to an increasing number of python minor releases. (Whether or not python attempts to treat stat fields other than dev_t as unsigned consistently has not been investigated.) I believe we were exposed to these errors when I removed our custom stat function in main: ae032d6 Drop our custom stat functions in favor of python's To date, bup has also required dev_t (and some other stat fields) to be unsigned and no more than eight bytes, via use of Q in the INDEX_SIG pack format for index entries. To address the dev_t issue, and to make sure we handle all the other stat fields consistently over time and across python versions, bring back and enhance our custom stat functions, and wrap python's os.mknod() to make sure it only sees the unsigned device values it expects (for now we don't add our own mknod because it's a bit more complex, and python's doesn't appear to have changed behavior over the periods we investigated). Have all of our stat operations work with the actual platform ranges (i.e. preserve unsigned/signed values), and make sure that the index doesn't lose the stat field sign information by providing the platform size and sign via _helpers.c_type_signed_size. Finally, so that we don't mishandle the index (even across hosts, say via moving a bup repo from one machine to another), add the entry signature to the index's header and require reindexing if the current platform's signature doesn't match the one in the index. Continue to support the previous index format (trivially) so that if the entry signature hasn't actually changed, you don't have to reindex just for the new format. Rename INDEX_SIG to ENTRY_SIG since that's what it is, and rename INDEX_HDR to INDEX_SIG since that's what *it* is. Add bup_int_from_py to pyutil and move the (u)int functions above bup_ulong_from_py. Thanks to Johannes Berg for a good bit of help figuring all this out. Signed-off-by: Rob Browning <rlb@defaultvalue.org> Tested-by: Rob Browning <rlb@defaultvalue.org>
rlbdv added a commit
to rlbdv-ci/bup
that referenced
this pull request
Jun 16, 2026
Tests on freebsd with zfs failed with OverflowErrors because zfs was creating a device (dev_t) with the high bit set (which it is known to), and it turns out that python hasn't handled that consistently. Recent python versions handle it as unsigned, ignoring the platform's definition in nearly all cases (e.g. on macos it's signed). Though it looks like os.makedev() is still broken. Python improved the "high bit set" dev_t handling in this commit (switching, as mentioned, to try to treat them as unsigned): 7111d9605f9db7aa0b095bb8ece7ccc0b8115c3f gh-89928: Fix integer conversion of device numbers (GH-31794) python/cpython#31794 And those changes have been backported to an increasing number of python minor releases. (Whether or not python attempts to treat stat fields other than dev_t as unsigned consistently has not been investigated.) I believe we were exposed to these errors when I removed our custom stat function in main: ae032d6 Drop our custom stat functions in favor of python's To date, bup has also required dev_t (and some other stat fields) to be unsigned and no more than eight bytes, via use of Q in the INDEX_SIG pack format for index entries. To address the dev_t issue, and to make sure we handle all the other stat fields consistently over time and across python versions, bring back and enhance our custom stat functions, and wrap python's os.mknod() to make sure it only sees the unsigned device values it expects (for now we don't add our own mknod because it's a bit more complex, and python's doesn't appear to have changed behavior over the periods we investigated). Have all of our stat operations work with the actual platform ranges (i.e. preserve unsigned/signed values), and make sure that the index doesn't lose the stat field sign information by providing the platform size and sign via _helpers.c_type_signed_size. Finally, so that we don't mishandle the index (even across hosts, say via moving a bup repo from one machine to another), add the entry signature to the index's header and require reindexing if the current platform's signature doesn't match the one in the index. Continue to support the previous index format (trivially) so that if the entry signature hasn't actually changed, you don't have to reindex just for the new format. Rename INDEX_SIG to ENTRY_SIG since that's what it is, and rename INDEX_HDR to INDEX_SIG since that's what *it* is. Add bup_int_from_py to pyutil and move the (u)int functions above bup_ulong_from_py. Thanks to Johannes Berg for a good bit of help figuring all this out. Signed-off-by: Rob Browning <rlb@defaultvalue.org> Tested-by: Rob Browning <rlb@defaultvalue.org>
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
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.
Fix os.major(), os.minor() and os.makedev().
Support device numbers larger than 2**63-1.
Support non-existent device number (NODEV).
https://bugs.python.org/issue45767