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

koobs · 2015-09-08T06:35:18Z

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

koobs · 2015-09-08T06:35:17Z

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

serhiy-storchaka · 2015-09-08T07:24:19Z

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).

koobs · 2015-09-08T07:29:51Z

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?

serhiy-storchaka · 2015-09-08T08:06:37Z

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.

koobs · 2015-09-08T09:19:24Z

@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.

serhiy-storchaka · 2015-09-08T09:36:52Z

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.

vadmium · 2015-09-08T10:54:55Z

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.

serhiy-storchaka · 2015-09-09T16:19:16Z

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

vadmium · 2017-06-15T03:44:28Z

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

corona10 · 2019-08-25T10:01:52Z

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

emaste · 2022-04-19T19:33:40Z

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

Use an 'unsigned long' instead of an 'unsigned int' for the request parameter of fcntl.ioctl() to support requests larger than UINT_MAX.

vstinner · 2024-05-24T09:51:45Z

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

Linux manual page of ioctl clearly uses the unsigned long type for the request parameter: https://man7.org/linux/man-pages/man2/ioctl.2.html

       int ioctl(int fd, unsigned long request, ...);

Python should use the right type. I wrote PR gh-119498 to fix the request parameter of fcntl.ioctl().

vstinner · 2024-05-24T09:56:29Z

FreeBSD uses unsigned long request: https://man.freebsd.org/cgi/man.cgi?ioctl
macOS uses unsigned long request: https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/ioctl.2.html
Solaris uses int request: https://docs.oracle.com/cd/E23824_01/html/821-1463/ioctl-2.html
POSIX uses int request: https://pubs.opengroup.org/onlinepubs/009695399/functions/ioctl.html

Linux, FreeBSD, macOS uses unsigned long. IMO we should take care of these platforms. Solaris is not supported by Python (PEP 11).

Use an 'unsigned long' instead of an 'unsigned int' for the request parameter of fcntl.ioctl() to support requests larger than UINT_MAX.

Use an 'unsigned long' instead of an 'unsigned int' for the request parameter of fcntl.ioctl() to support requests larger than UINT_MAX. (cherry picked from commit 92fab33) Co-authored-by: Victor Stinner <[email protected]>

Use an 'unsigned long' instead of an 'unsigned int' for the request parameter of fcntl.ioctl() to support requests larger than UINT_MAX. (cherry picked from commit 92fab33)

gh-69214: Fix fcntl.ioctl() request type (#119498) Use an 'unsigned long' instead of an 'unsigned int' for the request parameter of fcntl.ioctl() to support requests larger than UINT_MAX. (cherry picked from commit 92fab33)

gh-69214: Fix fcntl.ioctl() request type (GH-119498) Use an 'unsigned long' instead of an 'unsigned int' for the request parameter of fcntl.ioctl() to support requests larger than UINT_MAX. (cherry picked from commit 92fab33) Co-authored-by: Victor Stinner <[email protected]>

…GH-119498) (python#119504)" This reverts commit 0bab0b3.

…GH-119498) (python#119504)" This reverts commit 0bab0b3. The change modified how negative values, like termios.TIOCSWINSZ, was treated and is actually backward incompatible.

…#119498) (python#119505)" This reverts commit 078da88. The change modified how negative values, like termios.TIOCSWINSZ, was treated and is actually backward incompatible.

vstinner · 2024-06-01T14:09:33Z

This change broke test_ioctl on macOS: #119770

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

…) (#1… (#119905) Revert "[3.12] gh-69214: Fix fcntl.ioctl() request type (#119498) (#119505)" This reverts commit 078da88. The change modified how negative values, like termios.TIOCSWINSZ, was treated and is actually backward incompatible.

…9498) (… (#119906) Revert "[3.13] gh-69214: Fix fcntl.ioctl() request type (GH-119498) (#119504)" This reverts commit 0bab0b3. The change modified how negative values, like termios.TIOCSWINSZ, was treated and is actually backward incompatible.

Eclips4 · 2024-06-01T15:15:46Z

Thank you Victor for your work!

vstinner · 2024-06-01T20:00:00Z

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

Use an 'unsigned long' instead of an 'unsigned int' for the request parameter of fcntl.ioctl() to support requests larger than UINT_MAX.

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

ezio-melotti transferred this issue from another repository Apr 10, 2022

vstinner added the OS-freebsd label Apr 21, 2022

vstinner added a commit to vstinner/cpython that referenced this issue May 24, 2024

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

7bb6068

Use an 'unsigned long' instead of an 'unsigned int' for the request parameter of fcntl.ioctl() to support requests larger than UINT_MAX.

bedevere-app bot mentioned this issue May 24, 2024

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

Merged

vstinner changed the title ~~(FreeBSD/OSX) Fix fcntl module to accept 'unsigned long' type commands for ioctl(2).~~ Fix fcntl module to accept 'unsigned long' type commands for ioctl(2). May 24, 2024

vstinner added a commit that referenced this issue May 24, 2024

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

92fab33

Use an 'unsigned long' instead of an 'unsigned int' for the request parameter of fcntl.ioctl() to support requests larger than UINT_MAX.

This was referenced May 24, 2024

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

Merged

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

Merged

JelleZijlstra mentioned this issue May 28, 2024

backport a9a74da 3.13 #119642

Closed

vstinner added a commit to vstinner/cpython that referenced this issue Jun 1, 2024

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

517b639

…GH-119498) (python#119504)" This reverts commit 0bab0b3.

bedevere-app bot mentioned this issue Jun 1, 2024

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

Merged

bedevere-app bot mentioned this issue Jun 1, 2024

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

Merged

Eclips4 closed this as completed Jun 1, 2024

estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024

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

703410b

Use an 'unsigned long' instead of an 'unsigned int' for the request parameter of fcntl.ioctl() to support requests larger than UINT_MAX.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

koobs commented Sep 8, 2015 •

edited by bedevere-app bot

Loading

koobs commented Sep 8, 2015

serhiy-storchaka commented Sep 8, 2015

koobs commented Sep 8, 2015

serhiy-storchaka commented Sep 8, 2015

koobs commented Sep 8, 2015

serhiy-storchaka commented Sep 8, 2015

vadmium commented Sep 8, 2015

serhiy-storchaka commented Sep 9, 2015

vadmium commented Jun 15, 2017

corona10 commented Aug 25, 2019

emaste commented Apr 19, 2022

vstinner commented May 24, 2024

vstinner commented May 24, 2024

vstinner commented Jun 1, 2024

Eclips4 commented Jun 1, 2024

vstinner commented Jun 1, 2024

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

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

Comments

koobs commented Sep 8, 2015 • edited by bedevere-app bot Loading

Linked PRs

koobs commented Sep 8, 2015

serhiy-storchaka commented Sep 8, 2015

koobs commented Sep 8, 2015

serhiy-storchaka commented Sep 8, 2015

koobs commented Sep 8, 2015

serhiy-storchaka commented Sep 8, 2015

vadmium commented Sep 8, 2015

serhiy-storchaka commented Sep 9, 2015

vadmium commented Jun 15, 2017

corona10 commented Aug 25, 2019

emaste commented Apr 19, 2022

vstinner commented May 24, 2024

vstinner commented May 24, 2024

vstinner commented Jun 1, 2024

Eclips4 commented Jun 1, 2024

vstinner commented Jun 1, 2024

koobs commented Sep 8, 2015 •

edited by bedevere-app bot

Loading