classification
Title: mmap write segfaults if PROT_WRITE bit is not set in prot
Type: crash Stage: resolved
Components: Versions: Python 3.1, Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: neologix, pitrou
Priority: normal Keywords: patch

Created on 2011-03-03 22:20 by neologix, last changed 2011-03-06 01:25 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
mmap_check_prot_py3k.diff neologix, 2011-03-03 22:20 Patch and test against py3k review
Messages (6)
msg130008 - (view) Author: Charles-Fran├žois Natali (neologix) * (Python committer) Date: 2011-03-03 22:20
$ cat /tmp/test_mmap.py 
import mmap

m = mmap.mmap(-1, 1024, prot=mmap.PROT_READ|mmap.PROT_EXEC)
m[0] = 0
$ ./python /tmp/test_mmap.py 
Segmentation fault

When trying to perform a write, is_writable is called to check that we can indeed write to the mmaped area. is_writable just checks the access mode, and if it's not ACCESS_READ, we go ahead and proceed to the write.
The problem is that under Unix, it's possible to pass ACCESS_DEFAULT, and in that case no check is done on prot value.
In that case, is_writable will return true (since ACCESS_DEFAULT != ACCESS_READ), but if prot doesn't include PROT_WRITE bit, we'll segfault.
Attached is a patch including fix and specific test.
msg130010 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-03 22:32
Patch looks mostly good. Why do you use ~PROT_WRITE instead of PROT_READ|PROT_EXEC as in your example?
(I'm unsure whether a POSIX implementation could refuse such a value)
msg130032 - (view) Author: Charles-Fran├žois Natali (neologix) * (Python committer) Date: 2011-03-04 07:39
> Patch looks mostly good. Why do you use ~PROT_WRITE instead of PROT_READ|PROT_EXEC as in your example?

Because I'm not sure that PROT_EXEC is supported by all platforms. See
http://pubs.opengroup.org/onlinepubs/007908799/xsh/mmap.html :
"The implementation will support at least the following values of
prot: PROT_NONE, PROT_READ, PROT_WRITE, and the inclusive OR of
PROT_READ and PROT_WRITE."
If PROT_EXEC is defined but unsupported, it's likely to be defined as
0, so passing PROT_READ|PROT_EXEC will just pass PROT_READ (which is
catched by the current version), whereas with ~PROT_WRITE we're sure
that the PROT_WRITE bit won't be set.

> (I'm unsure whether a POSIX implementation could refuse such a value)

Me neither. I'd guess that the syscall just performs bitwise AND to
check the bits that are set, but you never know :-)
Maybe we could try this and see if a builbot complains ?
msg130134 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-06 00:22
Following error on the OpenIndiana buildbot:

test test_mmap failed -- Traceback (most recent call last):
  File "/export/home/buildbot/32bits/3.1.cea-indiana-x86/build/Lib/test/test_mmap.py", line 242, in test_access_parameter
    m = mmap.mmap(f.fileno(), mapsize, prot=~mmap.PROT_WRITE)
mmap.error: [Errno 13] Permission denied
msg130136 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-06 00:55
Buildbot failure should be fixed in e3eaf7dbb2b4.
msg130137 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-06 01:25
Merged in 92ab79ca4eeb, f9f9662dfb1f, 2e4468841c4c. Thank you!
History
Date User Action Args
2011-03-06 01:25:54pitrousetstatus: open -> closed
versions: + Python 3.1
nosy: pitrou, neologix
messages: + msg130137

resolution: fixed
stage: patch review -> resolved
2011-03-06 00:55:16pitrousetnosy: pitrou, neologix
messages: + msg130136
2011-03-06 00:22:10pitrousetnosy: pitrou, neologix
messages: + msg130134
2011-03-04 07:39:27neologixsetnosy: pitrou, neologix
messages: + msg130032
2011-03-03 22:32:02pitrousetversions: + Python 2.7, Python 3.2, Python 3.3
nosy: + pitrou

messages: + msg130010

type: crash
stage: patch review
2011-03-03 22:20:28neologixcreate