mmap write segfaults if PROT_WRITE bit is not set in prot

[neologix]

BPO	11391
Nosy	@pitrou
Files	mmap_check_prot_py3k.diff: Patch and test against py3k

^{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 2011-03-06.01:25:54.242>
created_at = <Date 2011-03-03.22:20:28.762>
labels = ['type-crash']
title = 'mmap write segfaults if PROT_WRITE bit is not set in prot'
updated_at = <Date 2011-03-06.01:25:54.240>
user = 'https://bugs.python.org/neologix'

bugs.python.org fields:

activity = <Date 2011-03-06.01:25:54.240>
actor = 'pitrou'
assignee = 'none'
closed = True
closed_date = <Date 2011-03-06.01:25:54.242>
closer = 'pitrou'
components = []
creation = <Date 2011-03-03.22:20:28.762>
creator = 'neologix'
dependencies = []
files = ['20991']
hgrepos = []
issue_num = 11391
keywords = ['patch']
message_count = 6.0
messages = ['130008', '130010', '130032', '130134', '130136', '130137']
nosy_count = 2.0
nosy_names = ['pitrou', 'neologix']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue11391'
versions = ['Python 3.1', 'Python 2.7', 'Python 3.2', 'Python 3.3']

[neologix]

$ 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.

[pitrou]

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)

[neologix]

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 ?

[pitrou]

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

[pitrou]

Buildbot failure should be fixed in e3eaf7dbb2b4.

[pitrou]

Merged in 92ab79ca4eeb, f9f9662dfb1f, 2e4468841c4c. Thank you!