Skip to content

ioctl request argument broken on 64-bit OpenBSD or OS X #45812

Description

@fbvortex
mannequin
BPO 1471
Nosy @loewis, @birkenfeld, @gpshead

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 = 'https://github.com/gpshead'
closed_at = <Date 2008-08-04.00:46:44.943>
created_at = <Date 2007-11-20.00:27:07.941>
labels = ['easy', 'type-bug']
title = 'ioctl request argument broken on 64-bit OpenBSD or OS X'
updated_at = <Date 2008-08-04.00:46:44.942>
user = 'https://bugs.python.org/fbvortex'

bugs.python.org fields:

activity = <Date 2008-08-04.00:46:44.942>
actor = 'gregory.p.smith'
assignee = 'gregory.p.smith'
closed = True
closed_date = <Date 2008-08-04.00:46:44.943>
closer = 'gregory.p.smith'
components = []
creation = <Date 2007-11-20.00:27:07.941>
creator = 'fbvortex'
dependencies = []
files = []
hgrepos = []
issue_num = 1471
keywords = ['patch', '64bit', 'easy']
message_count = 15.0
messages = ['57673', '57676', '57686', '57697', '57699', '58237', '63803', '63897', '63898', '64124', '64155', '70078', '70088', '70089', '70673']
nosy_count = 7.0
nosy_names = ['loewis', 'georg.brandl', 'gregory.p.smith', 'jafo', 'fbvortex', 'nicm', 'msavory']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue1471'
versions = ['Python 2.5']

Activity

  1. fbvortex commented on Nov 20, 2007

    fbvortexmannequin
    MannequinAuthor

    The following lines of code work on Linux platforms (amd64), and 32-bit
    OpenBSD (i386), but not on 64-bit OpenBSD platforms (at least not on
    amd64 or sparc64):

    import fcntl,os,pty,termios,select,sys,struct,pwd,signal,os
    pid,fd=pty.fork()
    fcntl.ioctl(fd, termios.TIOCSWINSZ, struct.pack("HHHH",80,25,0,0))

    This gives an "IOError: [Errno 25] Inappropriate ioctl for device"
    exception.

    The python version in question on OpenBSD is:

    Python 2.5.1 (r251:54863, Nov 16 2007, 18:16:44)
    [GCC 3.3.5 (propolice)] on openbsd4

    The winsize structure as dumped using sizeof(struct winsize) under
    OpenBSD/sparc64 is 8 bytes large, and so is the result from the
    struct.pack operation. Forcing the endianness big or little in the
    struct.pack formatting string has no effect on the error.

  2. fbvortex commented on Nov 20, 2007

    fbvortexmannequin
    MannequinAuthor

    The following C code, when compiled with -lutil runs without reporting
    any errors on both the sparc64 and i386 platforms on OpenBSD:

    #include <util.h>
    #include <utmp.h>
    #include <termios.h>
    #include <sys/ioctl.h>
    #include <stdio.h>
     
    int main(void)
    {
            int fd;
            struct winsize w;
     
            w.ws_row = 25;
            w.ws_col = 80;
            w.ws_xpixel = w.ws_ypixel = 0;
     
            forkpty(&fd, NULL, NULL, NULL);
            if (ioctl(fd,TIOCSWINSZ, &w) == -1)
                    perror("ioctl");
            return 0;
    }
  3. loewis commented on Nov 20, 2007

    loewismannequin
    Mannequin

    So what's the definition of struct winsize on these systems?

    Also, why do you think this is a bug in Python? AFAICT, the specific
    ioctl call does not occur in Python, but in your own code.

  4. nicm commented on Nov 20, 2007

    nicmmannequin
    Mannequin

    I can also reproduce this on OpenBSD/amd64 and was one of the people who
    discussed it with the submitter before he created this report.

    So what's the definition of struct winsize on these systems?

    The definition of struct winsize on both 32-bit and 64-bit OpenBSD
    platforms is:

    struct winsize {
            unsigned short  ws_row;         /* rows, in characters */
            unsigned short  ws_col;         /* columns, in characters */
            unsigned short  ws_xpixel;      /* horizontal size, pixels */
            unsigned short  ws_ypixel;      /* vertical size, pixels */
    };

    We have verified that (sizeof (struct winsize)) is 8 on all three
    platforms (32-bit i386, and 64-bit amd64 and sparc64).

    Also, why do you think this is a bug in Python? AFAICT, the specific
    ioctl call does not occur in Python, but in your own code.

    This Python code fails:

    fcntl.ioctl(fd, termios.TIOCSWINSZ, struct.pack("HHHH",80,25,0,0))

    (Error: IOError: [Errno 25] Inappropriate ioctl for device) This code
    also fails with the same error message (I would expect EINVAL instead):

    fcntl.ioctl(0, termios.TIOCSWINSZ, "")

    Corresponding test C code to call the ioctl (as listed in a previous
    message) works without problems on all three platforms. I don't know how
    Python fcntl.ioctl works and perhaps the problem is not Python, but I am
    having to try fairly hard to think what else could be involved.

    The constant appears to be the correct ioctl number. (Python code:

    python2.5 -c 'import termios; print "%d" % (termios.TIOCSWINSZ)'

    matches C code:

    #include <termios.h>
    int main(void) { printf("%ld\n", TIOCSWINSZ); }

    on all three platforms.)

    I am told it works on Linux. However, Linux declares ioctl as:

    int ioctl(int d, int request, ...); 

    So 'request' is 32-bits, but on OpenBSD ioctl is declared as:

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

    So on amd64 and sparc64, 'request' is 64-bits. Could this be a factor?

    -- Nicholas.

  5. nicm commented on Nov 20, 2007

    nicmmannequin
    Mannequin

    Okay, looks like my guess was correct. The diff at the end of this mail
    makes it work on OpenBSD/amd64 and it continues to work on i386, but it
    will probably break Linux (due to the problem it was working around
    mentioned in the comment at the beginning). I don't know enough about
    the Python build system to suggest a correct fix. (SUSv3 seems to
    suggest ioctl's request should be int, so perhaps OpenBSD is ultimately
    at fault.)

    fcntl_ioctl in fcntlmodule.c seems to have some other potential
    problems on 64-bit archs too, as it seems to coerce the ioctl argument
    (potentially a pointer) into an int.

    -- Nicholas.

    --- fcntlmodule.c.orig	Tue Nov 20 09:39:12 2007
    +++ fcntlmodule.c	Tue Nov 20 09:59:50 2007
    @@ -101,7 +101,7 @@
     	   the signed int 'code' variable, because Python turns 
    0x8000000
     	   into a large positive number (PyLong, or PyInt on 64-bit
     	   platforms,) whereas C expects it to be a negative int */
    -	int code;
    +	unsigned long code;
     	int arg;
     	int ret;
     	char *str;
    @@ -109,7 +109,7 @@
     	int mutate_arg = 1;
      	char buf[IOCTL_BUFSZ+1];  /* argument plus NUL byte */
     
    -	if (PyArg_ParseTuple(args, "O&Iw#|i:ioctl",
    +	if (PyArg_ParseTuple(args, "O&Lw#|i:ioctl",
                                  conv_descriptor, &fd, &code, 
     			     &str, &len, &mutate_arg)) {
     		char *arg;
    @@ -160,7 +160,7 @@
     	}
     
     	PyErr_Clear();
    -	if (PyArg_ParseTuple(args, "O&Is#:ioctl",
    +	if (PyArg_ParseTuple(args, "O&Ls#:ioctl",
                                  conv_descriptor, &fd, &code, &str, &len)) 
    {
     		if (len > IOCTL_BUFSZ) {
     			PyErr_SetString(PyExc_ValueError,
    @@ -182,7 +182,7 @@
     	PyErr_Clear();
     	arg = 0;
     	if (!PyArg_ParseTuple(args,
    -	     "O&I|i;ioctl requires a file or file descriptor,"
    +	     "O&L|i;ioctl requires a file or file descriptor,"
     	     " an integer and optionally an integer or buffer argument",
     			      conv_descriptor, &fd, &code, &arg)) {
     	  return NULL;
  6. msavory commented on Dec 6, 2007

    msavorymannequin
    Mannequin

    Similar issue seen in 64 bit OSX 10.5

    File "./ajaxterm.py", line 418, in create
    fcntl.ioctl(fd,
    struct.unpack('i',struct.pack('I',termios.TIOCSWINSZ))[0],
    struct.pack("HHHH",h,w,0,0))
    IOError: [Errno 25] Inappropriate ioctl for device

    $ python
    Python 2.5.1 (r251:54863, Oct  5 2007, 21:08:09) 
    [GCC 4.0.1 (Apple Inc. build 5465)] on darwin
  7. jafo commented on Mar 17, 2008

    jafomannequin
    Mannequin

    I think this proposed change needs some research into what the standards
    say about ioctl's return code, and if the change to a long return code
    is done then test it on Linux to see if it breaks things or if it works.
    Thoughts?

  8. nicm commented on Mar 18, 2008

    nicmmannequin
    Mannequin

    It's not the return value, it's the request argument (second argument).
    I thought SUSv3 had it as an int, but looking again its ioctl seems to
    be a) optional and b) all about STREAMS, so I don't think it applies:

    http://www.opengroup.org/onlinepubs/009695399/functions/ioctl.html

    -- Nicholas

  9. gpshead commented on Mar 18, 2008

    @gpshead
    Member

    i'd say the patch is fine.

    on linux ioctl takes an int.
    on openbsd it takes an unsigned long.
    on something else it might even take its own type like an ioctl_t.

    regardless, treating the parameter as either a long or unsigned long
    will work properly as that will cast downwards to the actual used
    parameter type correctly in the C code.

  10. self-assigned this
    on Mar 18, 2008
  11. changed the title [-]ioctl doesn't work properly on 64-bit OpenBSD[/-] [+]ioctl request argument broken on 64-bit OpenBSD or OS X[/+] on Mar 18, 2008
  12. gpshead commented on Mar 19, 2008

    @gpshead
    Member

    I am unable to reproduce this problem at all on Mac OS X 10.4 or 10.5
    with 32-bit or 64-bit python trunk builds.

    I have however checked in a fix that'll prevent negative code values
    passed in from being sign extended to 64-bits when converted to an
    unsigned long on platforms that use those.

    A unittest for fcntl.ioctl behavior when passed both positive and
    negative values is included.

    trunk r61652

    while the Linux man page describe's ioctl's code parameter as an int,
    the sys/ioctl.h file on linux has it as an unsigned long int in the
    function prototype. So its quite possible it is actually an unsigned
    long on all platforms and we should change unsigned int code to unsigned
    long code and the 'I' to a 'k'.

    However if that is done we -might- have a sign extension problem again
    if anyone is passing a negative value in and expecting it to act like a
    32-bit value. If the OS kernels only look at the lower 32-bits of the
    ioctl code during the syscall it'd work, but if it tested the 64bit
    value -123 64bit != -123 32bit. The better answer to that would be to
    just reject all negative values and fix any defined ioctl code constants
    to be positive on all platforms.

    fbvortex. Can you apply the change from svn to your python and test
    that it works properly on your 64-bit OpenBSD system? Thanks.

    paste a note here once you have with the results.

  13. gpshead commented on Mar 20, 2008

    @gpshead
    Member

    r61665 moves the test to test_ioctl instead of test_fcntl. it also
    forces it to only run when a pty is present rather than assuming fd 0

  14. birkenfeld commented on Jul 20, 2008

    @birkenfeld
    Member

    Something else to do here?

  15. gpshead commented on Jul 20, 2008

    @gpshead
    Member

    i believe this is fixed by the two changes mentioned above, i was
    waiting for fbvortex to confirm the fix on his 64-bit OpenBSD system.

    these changes need backporting to release25-maint.

  16. nicm commented on Jul 20, 2008

    nicmmannequin
    Mannequin

    I was going to test this on sparc64 (I no longer have access to amd64)
    but I've been busy/on holiday, I'll try to do it this week.

  17. gpshead commented on Aug 4, 2008

    @gpshead
    Member

    committed to release25-maint as r65466. if testing on 64bit openbsd or
    similar reveals that this is not fixed, please reopen the bug.

  18. transferred this issue fromon Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

easytype-bugAn unexpected behavior, bug, or error

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions