Skip to content

Fix fcntl module to accept 'unsigned long' type commands for ioctl(2). #69214

Closed
Closed
Fix fcntl module to accept 'unsigned long' type commands for ioctl(2).#69214
Labels
OS-freebsdeasyinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

Description

@koobs
koobs
opened on Sep 8, 2015
BPO 25026
Nosy @larryhastings, @ceronman, @vadmium, @serhiy-storchaka, @koobs, @corona10

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 2015-09-08.06:35:17.934>
labels = ['interpreter-core', 'easy', 'type-bug']
title = "(FreeBSD/OSX) Fix fcntl module to accept 'unsigned long' type commands for ioctl(2)."
updated_at = <Date 2019-08-25.10:01:51.693>
user = 'https://github.com/koobs'

bugs.python.org fields:

activity = <Date 2019-08-25.10:01:51.693>
actor = 'corona10'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2015-09-08.06:35:17.934>
creator = 'koobs'
dependencies = []
files = []
hgrepos = []
issue_num = 25026
keywords = ['easy']
message_count = 10.0
messages = ['250163', '250164', '250166', '250170', '250173', '250174', '250178', '250322', '296062', '350447']
nosy_count = 6.0
nosy_names = ['larry', 'ceronman', 'martin.panter', 'serhiy.storchaka', 'koobs', 'corona10']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue25026'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5', 'Python 3.6']

Linked PRs

Activity

  1. koobs commented on Sep 8, 2015

    @koobs
    koobs
    on Sep 8, 2015
    Author

    In my current attempt to create a FreeBSD port for python35, I've come across a patch rejection for the fcntlmodule.c for a local port patch we've been carrying since Python 2.6:

    https://svnweb.freebsd.org/ports/head/lang/python27/files/patch-Modules__fcntlmodule.c?revision=391238&view=markup

    The original commit log for this change is:

    ====================================

    Fix fcntl module to accept 'unsigned long' type commands for ioctl(2).

    Although POSIX says the type is 'int', all BSD variants (including Mac OS X)
    have been using 'unsigned long' type for very long time and its use predates
    the standard long enough. For certain commands (e.g., TIOCSWINSZ, FIONBIO),
    the Python value may get sign-extended on 64-bit platforms (by implicit type
    promotion) and it causes annoying warnings from kernel such as this:

    WARNING pid 24509 (python2.6): ioctl sign-extension ioctl ffffffff8004667e

    ====================================

    I'm not sure how this should be fixed upstream, nor clear on how to re-patch it given recent changes to fcntlmodule.c

  2. koobs
    added
    interpreter-core(Objects, Python, Grammar, and Parser dirs)
    easy
    type-bugAn unexpected behavior, bug, or error
    on Sep 8, 2015

  3. serhiy-storchaka commented on Sep 8, 2015

    @serhiy-storchaka
    serhiy-storchaka
    on Sep 8, 2015
    Member

    You need just replace unsigned_int with unsigned_long in Clinic declaration for fcntl.ioctl in Modules/fcntlmodule.c and regenerate Clinic code (make clinic).

  4. koobs commented on Sep 8, 2015

    @koobs
    koobs
    on Sep 8, 2015
    Author

    Thanks for the insight Serhiy. A few questions ..

    Is clinic code updated based on *.c declarations at build time? If not when/how is the best place/method to run this for our ports/package builds?

    Does your suggestion to switch unsigned_int to unsigned_long imply that the following lines are no longer necessary?

    • if (PyArg_ParseTuple(args, "O&Iw#|i:ioctl",
      + if (PyArg_ParseTuple(args, "O&kw#|i:ioctl",

    How do we get something like this fixed upstream for FreeBSD/OSX?

    If left un-patched, what is the impact on the user/system? Just a warning on console?

  5. serhiy-storchaka commented on Sep 8, 2015

    @serhiy-storchaka
    serhiy-storchaka
    on Sep 8, 2015
    Member

    No, the clinic code is not updated at build time. Your should either update clinic code just after patching Modules/fcntlmodule.c (Python 3 is needed), or update clinic code at patch creation time and include changes of generated clinic files in the patch.

    Generated clinic files contain a code for argument parsing (PyArg_ParseTuple...).

    I don't know if there is easy way to make this change conditionally for FreeBSD/OSX. The only way that I know is writing custom converters.

    Perhaps the original commit log is outdated. What was the type of code at the time of this log? Now it is unsigned int and this doesn't make sign extension when casted to unsigned long. While the code parameter is in the range 0..0xffffffff, unpatched code shouldn't have any visible effects.

  6. koobs commented on Sep 8, 2015

    @koobs
    koobs
    on Sep 8, 2015
    Author

    @serhiy

    If by "type of code at the time of commit" you mean upstream python code, msg250163 contains a link to what bits we replace in fcntlmodule.c and that "I -> k" has always been the same.

  7. serhiy-storchaka commented on Sep 8, 2015

    @serhiy-storchaka
    serhiy-storchaka
    on Sep 8, 2015
    Member

    Originally the type of the code variable in fcntl_ioctl() was int. In cbad1f5cabb1 it was changed to unsigned int. I guess that the log message that you cited was written before this.

  8. vadmium commented on Sep 8, 2015

    @vadmium
    vadmium
    on Sep 8, 2015
    Member

    If necessary, perhaps we could unconditionally change the Python-level argument to unsigned_long, and then add a conditional bit in the C code to convert it to int if appropriate.

    But I wonder if most of the problem is fixed by bpo-1471 (linked from the commit Serhiy identified). As I see it, your patch should now only be needed to support “code” values outside of the range of unsigned_int, e.g. that require > 32 bits.

  9. serhiy-storchaka commented on Sep 9, 2015

    @serhiy-storchaka
    serhiy-storchaka
    on Sep 9, 2015
    Member

    The question is: are ioctl codes outside of the unsigned int range used on BSD family or Mac OS X?

  10. vadmium commented on Jun 15, 2017

    @vadmium
    vadmium
    on Jun 15, 2017
    Member

    Maybe bpo-16124 is related; it mentions 64-bit values, but it sounds like an obscure use case.

  11. corona10 commented on Aug 25, 2019

    @corona10
    corona10
    on Aug 25, 2019
    Member

    Can I take a look at this issue?
    Is there anything should I care about except update clinic?
    Thanks!

  12. ezio-melotti
    transferred this issue fromon Apr 10, 2022

  13. emaste commented on Apr 19, 2022

    @emaste
    emaste
    on Apr 19, 2022
    Contributor

    bpo-16124 is #60328

    The question is: are ioctl codes outside of the unsigned int range used on BSD family or Mac OS X?

    I don't have an authoritative answer for other BSDs or OS X, but for FreeBSD they are not. In addition, we have hidden the warning under a diagnostic option in freebsd/freebsd-src@a90fb6c

  14. 5 remaining items

  15. vstinner
    added a commit that references this issue on May 24, 2024

    gh-69214: Fix fcntl.ioctl() request type (#119498)

    92fab33
  16. miss-islington
    added a commit that references this issue on May 24, 2024

    pythongh-69214: Fix fcntl.ioctl() request type (pythonGH-119498)

    bd62cde
  17. vstinner
    added 3 commits that reference this issue on May 24, 2024

    pythongh-69214: Fix fcntl.ioctl() request type (python#119498)

    729941c

    [3.12] gh-69214: Fix fcntl.ioctl() request type (#119498) (#119505)

    078da88

    [3.13] gh-69214: Fix fcntl.ioctl() request type (GH-119498) (#119504)

    0bab0b3
  18. vstinner
    added 3 commits that reference this issue on Jun 1, 2024

    Revert "[3.13] pythongh-69214: Fix fcntl.ioctl() request type (python…

    517b639

    Revert "[3.13] pythongh-69214: Fix fcntl.ioctl() request type (python…

    63ae381

    Revert "[3.12] pythongh-69214: Fix fcntl.ioctl() request type (python…

    965b4b7

  19. vstinner commented on Jun 1, 2024

    @vstinner
    vstinner
    on Jun 1, 2024
    Member

    This change broke test_ioctl on macOS: #119770

    So I created two PRs to revert the change in 3.12 and 3.13 branches.

  20. vstinner
    added 2 commits that reference this issue on Jun 1, 2024

    [3.12] Revert "[3.12] gh-69214: Fix fcntl.ioctl() request type (#119498…

    34a0e7a

    [3.13] Revert "[3.13] gh-69214: Fix fcntl.ioctl() request type (GH-11…

    4e147ca

  21. Eclips4 commented on Jun 1, 2024

    @Eclips4
    Eclips4
    on Jun 1, 2024
    Member

    Thank you Victor for your work!

  22. Eclips4
    closed this as completedon Jun 1, 2024

  23. vstinner commented on Jun 1, 2024

    @vstinner
    vstinner
    on Jun 1, 2024
    Member

    I went through FreeBSD issues recently and decided to fix this 9 years old issue.

  24. estyxx
    added a commit that references this issue on Jul 17, 2024

    pythongh-69214: Fix fcntl.ioctl() request type (python#119498)

    703410b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    OS-freebsdeasyinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions