Skip to content

pass big values for arg to fcntl.ioctl #74850

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ukl mannequin opened this issue Jun 14, 2017 · 9 comments
Closed

pass big values for arg to fcntl.ioctl #74850

ukl mannequin opened this issue Jun 14, 2017 · 9 comments
Assignees
Labels
3.14 new features, bugs and security fixes extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error

Comments

@ukl
Copy link
Mannequin

ukl mannequin commented Jun 14, 2017

BPO 30665
Nosy @Yhg1s, @mdickinson, @vstinner, @serhiy-storchaka
PRs
  • bpo-30665: fcntl.ioctl() now accepts larger range of integers #2286
  • Files
  • unsigned-long-arg-for-ioctl.patch
  • 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 2017-06-14.14:27:52.134>
    labels = ['type-bug', '3.9', '3.10']
    title = 'pass big values for arg to fcntl.ioctl'
    updated_at = <Date 2020-05-28.01:38:07.976>
    user = 'https://bugs.python.org/ukl'

    bugs.python.org fields:

    activity = <Date 2020-05-28.01:38:07.976>
    actor = 'ned.deily'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = []
    creation = <Date 2017-06-14.14:27:52.134>
    creator = 'ukl'
    dependencies = []
    files = ['46951']
    hgrepos = []
    issue_num = 30665
    keywords = ['patch']
    message_count = 4.0
    messages = ['296008', '296242', '296307', '296405']
    nosy_count = 6.0
    nosy_names = ['twouters', 'mark.dickinson', 'vstinner', 'serhiy.storchaka', 'ukl', 'cfi']
    pr_nums = ['2286']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue30665'
    versions = ['Python 3.9', 'Python 3.10']

    @ukl
    Copy link
    Mannequin Author

    ukl mannequin commented Jun 14, 2017

    When passing a big integer value to fcntl.ioctl this might result in

     OverflowError: signed integer is greater than maximum
    

    . Traditionally ioctl(3) takes a pointer as third argument. The fcntl module however uses an int (format specifier 'i') which is too small to hold a pointer on 64 bit architectures.

    The fix is to use 'k' (and an unsigned long) instead of 'i' (and an int). An unsigned long is suitable to hold a pointer value on AFAIK all platforms that matter today. (And it works on all platforms where int is already good enough, so the suggested change doesn't do any harm.)

    A patch is attached.

    @ukl ukl mannequin added the type-bug An unexpected behavior, bug, or error label Jun 14, 2017
    @serhiy-storchaka
    Copy link
    Member

    The FreeBSD [1], OpenBSD [2], and Solaris 10 [3] documentations specify the third argument of ioctl() as either an int or a pointer.

    Passing a 64-bit long instead of a 32-bit int on big-endian platform can cause incorrect interpretation of an argument.

    [1] https://www.freebsd.org/cgi/man.cgi?query=ioctl&sektion=2
    [2] http://man.openbsd.org/ioctl.2
    [3] https://docs.oracle.com/cd/E26505_01/html/816-5167/ioctl-2.html

    @ukl
    Copy link
    Mannequin Author

    ukl mannequin commented Jun 19, 2017

    So the only option to fix this is to determine the type of arg from the request parameter? I failed to find the implementation of ioctl for linux in glibc, the best I found is one that only seems to be used on powerpc[1] which seems to assume that the third argument is a pointer. The Linux kernel prototype takes an unsigned long[2] and encodes the size of the data pointed to in the request using _IOC[3]. On hurd there are different macros to determine the type. I'm not fluent in *BSD and Solaris (and other operating systems), so I cannot add specifics for them. I cannot even say which OS that runs Python can run in 64 bit BE mode.

    [1] https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/powerpc/ioctl.c;h=e2e3d3357f9f5ee9ffe5e9f0588ebae9976c0dfd;hb=HEAD
    [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/ioctl.c?h=v4.12-rc5#n691
    [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/asm-generic/ioctl.h

    @serhiy-storchaka
    Copy link
    Member

    PR 2286 uses for the third argument char* on Linux and int or unsigned int on other platforms. It keeps some degree of integer overflow control.

    @serhiy-storchaka serhiy-storchaka added the 3.7 (EOL) end of life label Jun 20, 2017
    @ned-deily ned-deily added 3.9 only security fixes 3.10 only security fixes and removed 3.7 (EOL) end of life labels May 28, 2020
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @iritkatriel iritkatriel added the extension-modules C modules in the Modules dir label Nov 28, 2023
    @serhiy-storchaka
    Copy link
    Member

    My PR also is not correct, because even on Linux some calls require an argument of type int (e.g. file descriptor).

    It is not possible safely pass integer arguments in wider range without introducing an explicit parameter for this. But what is the use case? What calls require an integer argument in wider range? If it needs a pointer, it is usually a pointer to some request-specific structure, and you can pass a buffer object that represent such structure. The address of the buffer will be passed to the ioctl() system call. It is currently not possible to pass an arbitrary pointer.

    For now, int seems working well in existing code. If there are concrete examples for integer arguments in the wider range or arbitrary pointers, we will try to find different solution.

    @serhiy-storchaka serhiy-storchaka added pending The issue will be closed if no feedback is provided 3.13 bugs and security fixes and removed 3.10 only security fixes 3.9 only security fixes labels Dec 24, 2023
    @picnixz
    Copy link
    Member

    picnixz commented Dec 2, 2024

    Do you mind closing this issue for now? @serhiy-storchaka or do you think this should be kept opened? (in which case, maybe remove the pending label)

    @serhiy-storchaka
    Copy link
    Member

    Some ioctl calls need int64_t instead of int. This is a part of a bigger issue (there are several open issues for other particular cases). I have a plan of a general solution, so I'll keep this issue open as a reminder.

    @serhiy-storchaka serhiy-storchaka self-assigned this Dec 2, 2024
    @serhiy-storchaka serhiy-storchaka added 3.14 new features, bugs and security fixes and removed pending The issue will be closed if no feedback is provided 3.13 bugs and security fixes labels Dec 2, 2024
    @serhiy-storchaka
    Copy link
    Member

    I may have been wrong, or at least I can't remember what I meant. Some fcntl() operations need a int64* argument, they should be handled by passing a 4-bytes buffer. I cannot recall any ioctl() operation that takes other integer than int.

    It may be that some values should actually be interpreted as unsigned int instead of int. In C, such conversion is transparent.

    @serhiy-storchaka
    Copy link
    Member

    I am closing this. If there are any constants that exceed the 32-bit integer range, we can re-open this issue.

    @serhiy-storchaka serhiy-storchaka closed this as not planned Won't fix, can't repro, duplicate, stale Apr 24, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.14 new features, bugs and security fixes extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants