GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — mmap lacks error handling (SEH) on Windows which can lead to interpreter crashes · Issue #118209 · python/cpython

mmap lacks error handling (SEH) on Windows which can lead to interpreter crashes #118209

New issue
Copy link
New issue
Copy link
Closed
Closed
mmap lacks error handling (SEH) on Windows which can lead to interpreter crashes#118209
Copy link
Labels
OS-windowsextension-modulesC modules in the Modules dirC modules in the Modules dirtype-crashA hard crash of the interpreter, possibly with a core dumpA hard crash of the interpreter, possibly with a core dump

Description

@Dobatymo
Dobatymo
opened on Apr 24, 2024
Issue body actions

Crash report

What happened?

mmap reads and writes require structured exception handling (SEH) for correct handling of errors (like hardware read issues, or disk full write issues) on Windows. See https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-mapviewoffile#remarks for a description of the possible exceptions and https://learn.microsoft.com/en-us/windows/win32/memory/reading-and-writing-from-a-file-view for how to handle them.

I found this issue when trying to read a file from a corrupt hdd. A normal open().read() would raise a OSError: [Errno 22] Invalid argument exception. Whereas doing the same using mmap crashes the interpreter.

I only tested on Windows 7 and Python 3.8, but from looking at the Microsoft docs and the Python source code, this issue should exist for all Windows versions and the latest Python source. (I don't have access to the broken file from a newer machine)

I think there is nothing here which guards against these errors

cpython/Modules/mmapmodule.c

Lines 294 to 313 in 2584082

static PyObject *
mmap_read_method(mmap_object *self,
PyObject *args)
{
Py_ssize_t num_bytes = PY_SSIZE_T_MAX, remaining;
PyObject *result;
CHECK_VALID(NULL);
if (!PyArg_ParseTuple(args, "|O&:read", _Py_convert_optional_to_ssize_t, &num_bytes))
return NULL;
CHECK_VALID(NULL);
/* silently 'adjust' out-of-range requests */
remaining = (self->pos < self->size) ? self->size - self->pos : 0;
if (num_bytes < 0 || num_bytes > remaining)
num_bytes = remaining;
result = PyBytes_FromStringAndSize(&self->data[self->pos], num_bytes);
self->pos += num_bytes;
return result;
}

with open(path, "rb") as fp:
    with mmap.mmap(fp.fileno(), 0, access=mmap.ACCESS_READ) as fr:
        fr.read(1024*8)   # this crashes when `path` is on a corrupt hdd for example.

CPython versions tested on:

3.8

Operating systems tested on:

Windows

Output from running 'python -VV' on the command line:

No response

Linked PRs

  • gh-118209: Add structured exception handling to mmap module #118213
  • [3.13] gh-118209: Add Windows structured exception handling to mmap module (GH-118213) #118887

Activity

  1. Dobatymo
    added
    type-crashA hard crash of the interpreter, possibly with a core dumpA hard crash of the interpreter, possibly with a core dump
    on Apr 24, 2024
  2. Eclips4
    added
    OS-windows
    extension-modulesC modules in the Modules dirC modules in the Modules dir
    on Apr 24, 2024
  3. zooba commented on Apr 24, 2024

    @zooba
    zooba
    on Apr 24, 2024
    Member
    More actions

    Posted this on the PR, but repeating here:

    One other possibility may be to protect PyBytes_FromString internally, which could cut off an entire class of crashes instead of just one. Thoughts?

  4. eryksun commented on Apr 24, 2024

    @eryksun
    eryksun
    on Apr 24, 2024
    Contributor
    More actions

    On POSIX, the mmap type doesn't currently support handling synchronous SIGSEGV or SIGBUS signals generated by an error while reading or writing in the address range of a memory-mapped file. That requires a handler set by sigaction() that's configured with SA_SIGINFO in order to be able to check whether the faulting si_addr is in the range of the mapped file.

    Is it premature to implement SEH or VEH support for this on Windows, given nothing similar is currently implemented on POSIX?

  5. zooba commented on Apr 24, 2024

    @zooba
    zooba
    on Apr 24, 2024
    Member
    More actions

    Is it premature to implement SEH or VEH support for this on Windows, given nothing similar is currently implemented on POSIX?

    Something has to be implemented first, and the likelihood of finding someone capable of doing both is pretty slim.

    I think we're best to implement one first, but in a way that's easily adapted to support other platforms (e.g. my safe_memcpy suggestion on the PR). That way another contributor has the framework for adding platform support, and we don't have to force collaboration or conflict resolution too early.

    Reacted by Eryk Sun
  6. zooba
    added a commit that references this issue on May 10, 2024

    gh-118209: Add Windows structured exception handling to mmap module (G…

    e85e8de
  7. miss-islington
    added a commit that references this issue on May 10, 2024

    pythongh-118209: Add Windows structured exception handling to mmap mo…

    f65258f
  8. zooba
    added a commit that references this issue on May 10, 2024

    gh-118209: Add Windows structured exception handling to mmap module (G…

    7dc9875
  9. zooba commented on May 10, 2024

    @zooba
    zooba
    on May 10, 2024
    Member
    More actions

    Thanks for the contribution! This is now merged into 3.13 for beta 2.

    If someone would like to add POSIX/signal support, please feel free, but let's do it on a new issue. Similarly with any additional operations that we can protect using SEH.

  10. zooba
    closed this as completedon May 10, 2024
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-windowsextension-modulesC modules in the Modules dirC modules in the Modules dirtype-crashA hard crash of the interpreter, possibly with a core dumpA hard crash of the interpreter, possibly with a core dump

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions

      You are visitor 01402503 — GeezerVison does not store the pages you look up.
      Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.