[Python-bugs-list] [ python-Bugs-585792 ] Invalid mmap crashes Python interpreter
SourceForge.net
noreply@sourceforge.net
Wed, 09 Apr 2003 16:23:05 -0700
Bugs item #585792, was opened at 2002-07-24 18:52
Message generated for change (Comment added) made by liqx
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=585792&group_id=5470
Category: Extension Modules
Group: None
Status: Closed
Resolution: Accepted
Priority: 5
Submitted By: paul rubin (phr)
Assigned to: Neal Norwitz (nnorwitz)
Summary: Invalid mmap crashes Python interpreter
Initial Comment:
This is under Red Hat 7.2, Linux kernel 2.4.7. "x" is
an empty file, i.e. zero bytes long.
import mmap
f = open(x,"r+")m = mmap.mmap(f.fileno(), 10)
print m[1]
Crashes Python with a bus error. Python should catch
the bus error signal
and raise an appropriate exception that the script can
catch.
----------------------------------------------------------------------
Comment By: Alastair (LiQUiDx) Tse (liqx)
Date: 2003-04-10 09:23
Message:
Logged In: YES
user_id=338973
i'm having some problems with this patch.
how does one use mmap on a device file with on a unix device
file?
# example - with a webcam on /dev/video0:
import os, mmap
fd = os.open("/dev/video0",os.O_RDONLY)
mm = mmap.mmap(fd, 1024, mmap.MAP_SHARED)
# this will result in:
Traceback (most recent call last):
File "<stdin>", line 1, in ?
ValueError: mmap length is greater than file size
even though this is a perfectly valid way to access the
buffer for video devices on linux (not sure about other unicies)
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-09-06 08:33
Message:
Logged In: YES
user_id=33168
Sorry Martin, I apparently didn't upload the last patch.
Feel free to fix the test if you see a problem.
Checked in as:
Modules/mmapmodule.c: 2.41, 2.35.6.3
Lib/test/test_mmap.py: 1.24, 1.19.8.3
Lib/test/output/test_mmap: 1.9, 1.8.6.1
Misc/NEWS: 1.480, 1.337.2.4.2.34
I backported the fix, since your last message Guido seemed
to indicate that we are still supposed to do it.
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2002-08-18 03:09
Message:
Logged In: YES
user_id=21627
I may be missing something: How precisely does this
special-case Windows in the test?
Apart from that, the patch is fine.
I'm uncertain on backporting it, because I'm uncertain how
backporting works at all since the PBF got instantiated. So
I'd suggest to mark this as a backport candidate, and let
the patch czar decide.
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-08-18 02:08
Message:
Logged In: YES
user_id=33168
Martin, would you like one last review and test on windows
before I check in? Also, should this be backported to 2.2.?
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-08-14 03:39
Message:
Logged In: YES
user_id=33168
I can't test windows. I've attached a patch which tries to
test windows doesn't raise an exception.
I also modified the NEWS to state this change affects
non-Windows platforms.
Tim/Martin, can you test (or at least review the test code)
that this works on Windows before I check it in? Or let me
know if you want me to check it in. It should be easy to
fix if it doesn't work on Windows.
Thanks.
----------------------------------------------------------------------
Comment By: Nobody/Anonymous (nobody)
Date: 2002-08-13 12:11
Message:
Logged In: NO
I'd say the following is reasonable:
- on mmap object creation, stat the file and check its size
against the size arg passed to mmap. Signal an error if the
file is smaller than the requested mmap size.
- On access, compare the subscript to the mmap object size
and signal an error if out of bounds (presumably this is
done already)
- Figure out if another process can truncate the file while
the mmap is alive. If yes, maybe add a method to the mmap
object that re-stats the file and makes sure it's still
larger than the mmap size.
- Add warnings to the mmap documentation that weirdness can
result from the file size changing etc.
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2002-08-13 02:31
Message:
Logged In: YES
user_id=21627
I think we should special-case Windows, then. For the
mmapmodule patch proper, nothing needs to be done; for the
testsuite test, pass and fail are exactly reversed on
Windows (but then, we'd test the implementation of Windows,
so skipping that test sounds reasonable as well).
With those changes, the patch is fine.
----------------------------------------------------------------------
Comment By: Tim Peters (tim_one)
Date: 2002-08-13 01:59
Message:
Logged In: YES
user_id=31435
mmap() on WIndows doesn't care if you map more bytes
than exist in the file -- if you do, it grows the file to match.
"""
If an application specifies a size for the file mapping object
that is larger than the size of the actual named file on disk,
the file on disk is grown to match the specified size of the
file mapping object. If the file cannot be grown, this results
in a failure to create the file mapping object. GetLastError
will return ERROR_DISK_FULL.
"""
Like so:
>>> import mmap
>>> import os
>>> os.path.getsize('abc.abc')
12L
>>> f = file('abc.abc', 'r+')
>>> m = mmap.mmap(f.fileno(), 10000)
>>> m.close()
>>> os.path.getsize('abc.abc')
10000L
>>>
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-08-13 01:49
Message:
Logged In: YES
user_id=33168
That last comment was supposed to be: I don't know how
Windows behaves...
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-08-13 01:47
Message:
Logged In: YES
user_id=33168
I thought the st_size was updated by the code in
new_buffersize. It was only the position, so the comment
should go. ValueError is fine with me. Should I add an
entry to NEWS? New patch attached w/NEWS and test.
There is another issue--Windows. I how Windows behaves and
I can't test it.
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2002-08-13 00:45
Message:
Logged In: YES
user_id=21627
I'd say it is a ValueError if the map size is larger than
the file size, instead of silently decreasing the size:
Errors should never pass silently, Unless explicitly silenced.
On the trick of new_buffersize: I'm not sure I understand
what you are referring to: we don't use stdio in mmap at all.
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-08-12 23:30
Message:
Logged In: YES
user_id=33168
I wasn't sure if I could change the size. For Solaris at
least, it seems this is the right thing to do. Is it likely
that other systems allow the file size to change? I've
forced the size to be the max(size_arg, size_of_file). This
has the added benefit of working for writing too. Prior to
this patch, writing past the end crashed on Linux.
There are 2 comments in the patch: one worrying about the
file changing, the other producing a warning message if the
size argument is larger than the file. Do we have to worry
about the size changing? Should we tell the user what they did?
Sorry about the old style diff. I remembered to do the cvs
-f diff part, but forgot the -C5. My defaults are unified.
New context patch attached.
----------------------------------------------------------------------
Comment By: Nobody/Anonymous (nobody)
Date: 2002-08-12 17:45
Message:
Logged In: NO
Please don't even think of adding a stat call to every
access. The whole idea of mmap is to speed up access by
eliminating system calls when you touch the file contents.
Checking the file size at map creation is reasonable. Some
note should also be added to the mmap doc about possible
crashes if the file size changes.
>From a Python script writer's viewpoint, the most convenient
fix is for Python to catch the bus error signal and inspect
the stack to figure out that the trap occurred in the mmap
routine, then generate and return an appropriate Python
exception. However, doing that is pretty
platform-dependent, and attempting it in all the Python
ports that support mmap doesn't sound practical.
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2002-08-12 17:15
Message:
Logged In: YES
user_id=21627
Ah, an old-style diff. I assume that goes into mmap_item?
I'd rather avoid the stat call on every access, and check
the size on map creation.
The Solaris man page says this about changing files:
If the size of the mapped file changes after the
call to
mmap() as a result of some other operation on the
mapped
file, the effect of references to portions of the
mapped
region that correspond to added or removed portions
of the
file is unspecified.
So it appears that this is a case of "don"t do this, then",
and that it atleast won't crash - but we might want to
perform a few experiments.
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-08-12 01:05
Message:
Logged In: YES
user_id=33168
Martin, could you please review the patch?
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-07-29 01:00
Message:
Logged In: YES
user_id=33168
Attached is a patch which fixes this problem. I'm not sure
this patch it correct nor appropriate. But it stops the
crash. I'm hoping some mmap expert can shed some light on this.
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=585792&group_id=5470