Skip to content
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

Random stack corruption from socketmodule.c #39819

Closed
mikesfpy mannequin opened this issue Jan 14, 2004 · 6 comments
Closed

Random stack corruption from socketmodule.c #39819

mikesfpy mannequin opened this issue Jan 14, 2004 · 6 comments
Labels
stdlib Python modules in the Lib dir

Comments

@mikesfpy
Copy link
Mannequin

mikesfpy mannequin commented Jan 14, 2004

BPO 876637
Files
  • sock_crash.py: sock_crash.py
  • 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 = <Date 2006-02-12.06:19:41.000>
    created_at = <Date 2004-01-14.06:41:12.000>
    labels = ['library']
    title = 'Random stack corruption from socketmodule.c '
    updated_at = <Date 2006-02-12.06:19:41.000>
    user = 'https://bugs.python.org/mikesfpy'

    bugs.python.org fields:

    activity = <Date 2006-02-12.06:19:41.000>
    actor = 'nnorwitz'
    assignee = 'nnorwitz'
    closed = True
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2004-01-14.06:41:12.000>
    creator = 'mikesfpy'
    dependencies = []
    files = ['1170']
    hgrepos = []
    issue_num = 876637
    keywords = []
    message_count = 6.0
    messages = ['19683', '19684', '19685', '19686', '19687', '19688']
    nosy_count = 5.0
    nosy_names = ['nnorwitz', 'troels', 'mikesfpy', 'arkadini', 'scott.dial']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue876637'
    versions = ['Python 2.4']

    @mikesfpy
    Copy link
    Mannequin Author

    mikesfpy mannequin commented Jan 14, 2004

    THE PROBLEM:

    The implementation of the socket_object.settimeout() method
    (socketmodule.c, function internal_select()) uses the select() system
    call with an unbounded file descriptor number. This will cause random
    stack corruption if fd>=FD_SETSIZE.

    This took me ages to track down! It happened with a massively
    multithreaded
    and massively connection-swamped network server. Basically most of
    the
    descriptors did not use that routine (because they were either pure
    blocking
    or pure non-blocking). But one module used settimeout() and with a little
    bit of luck got an fd>=FD_SETSIZE and with even more luck corrupted
    the
    stack and took down the whole server process.

    Demonstration script appended.

    THE SOLUTION:

    The solution is to use poll() and to favour poll() even if select()
    is available on a platform. The current trend in modern OS+libc
    combinations is to emulate select() in libc and call kernel-level poll()
    anyway. And this emulation is costly (both for the caller and for libc).

    Not so the other way round (only some systems of historical interest
    do that BTW), so we definitely want to use poll() if it's available
    (even if it's an emulation).

    And if select() is your only choice, then check for FD_SETSIZE before
    using the FD_SET macro (and raise some strange exception if that fails).

    [
    I should note that using SO_RCVTIMEO and SO_SNDTIMEO would be a lot
    more
    efficient (kernel-wise at least). Unfortunately they are not universally
    available (though defined by most system header files). But a simple
    runtime test with a fallback to poll()/select() would do.
    ]

    A PATCH, A PATCH?

    Well, the check for FD_SETSIZE is left as an exercise for the reader. :-)
    Don't forget to merge this with the stray select() way down by adding
    a return value to internal_select().

    But yes, I can do a 'real' patch with poll() [and even one with the
    SO_RCVTIMEO trick if you are adventurous]. But, I can't test it with
    dozens of platforms, various include files, compilers and so on.

    So, dear Python core developers: Please discuss this and tell me,
    if you want a patch, then you'll get one ASAP.

    Thank you for your time!

    @mikesfpy mikesfpy mannequin closed this as completed Jan 14, 2004
    @mikesfpy mikesfpy mannequin assigned nnorwitz Jan 14, 2004
    @mikesfpy mikesfpy mannequin added the stdlib Python modules in the Lib dir label Jan 14, 2004
    @mikesfpy mikesfpy mannequin closed this as completed Jan 14, 2004
    @mikesfpy mikesfpy mannequin assigned nnorwitz Jan 14, 2004
    @mikesfpy mikesfpy mannequin added the stdlib Python modules in the Lib dir label Jan 14, 2004
    @troels
    Copy link
    Mannequin

    troels mannequin commented Jun 10, 2004

    Logged In: YES
    user_id=32863

    I have created a patch to make socketmodule use poll() when
    available. See http://python.org/sf/970288

    (I'm not allowed to attach patches to this bug item.)

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Feb 7, 2006

    Logged In: YES
    user_id=33168

    Thanks!

    Committed revision 42253.
    Committed revision 42254. (2.4)

    @arkadini
    Copy link
    Mannequin

    arkadini mannequin commented Feb 9, 2006

    Logged In: YES
    user_id=1346917

    Unfortunately r42253 breaks things on win32 (at least on my
    machine).

    By default FD_SETSIZE is 64 (winsock.h, winsock2.h). Even if
    the check from select module WAS included, that would end up
    in 512. Most of the time, first socket I create after
    starting python gets a fd > 900.

    What is a reasonable value for FD_SETSIZE then? 1024?
    Compared to the default value of 64 doesn't look that
    reasonable, though...

    Are there plans to "elaborate" on the last fix?

    @scottdial
    Copy link
    Mannequin

    scottdial mannequin commented Feb 10, 2006

    Logged In: YES
    user_id=383208

    The patch that has been applied has no relevance to Windows
    and should not be used when compiling for Windows. The use
    of an internal counter to assign descriptors to the fd_set
    array avoids the problem noted here so there is no such bug
    on Windows. Futhermore, as Arek notes, Windows creates
    descriptor numbers with no bound.

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Feb 12, 2006

    Logged In: YES
    user_id=33168

    Martin has fixed my Window's breakage by only checking for
    selectability on non-Windows platforms.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    0 participants