This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Negative numbers to os.read() cause segfault
Type: Stage:
Components: Interpreter Core Versions: Python 2.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: mwh Nosy List: exarkun, foom, gerrit, mwh, quiver, rhettinger
Priority: normal Keywords:

Created on 2004-12-01 21:40 by exarkun, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
os-read-oopsie.diff mwh, 2004-12-04 20:38 mwh's fix #1
Messages (11)
msg23510 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2004-12-01 21:40
Python 2.3.4 (#2, Sep 24 2004, 08:39:09)
[GCC 3.3.4 (Debian 1:3.3.4-12)] on linux2
Type "help", "copyright", "credits" or "license" for
more information.
>>> import sys, os
>>> stdin = sys.stdin.fileno()
>>> os.read(stdin, 0)
''
>>> os.read(stdin, 0)
''
>>> os.read(stdin, -1)
asdkljasd
'asdk\x01\x00\x00\x00\x00\x00'
>>> os.read(stdin, 100)
Segmentation fault
exarkun@boson:~$

This problem persists in Python 2.4, although the
resulting incorrect behavior differs slightly (at least
on my build), as is to be expected of a memory
corrupting bug.

Note that the value returned from os.read(stdin, -1) is
incorrect in addition to the subsequent read segfaulting.
msg23511 - (view) Author: James Y Knight (foom) Date: 2004-12-01 23:11
Logged In: YES 
user_id=1104715

This appears to be because PyString_FromStringAndSize takes a signed int 
for size, doesn't verify that it is > 0, and then adds it to 
sizeof(PyStringObject):
    op = (PyStringObject *)PyObject_MALLOC(sizeof(PyStringObject) + 
size);

PyObject_MALLOC will fail if given a < 0 size, but, if size is > 
-sizeof(PyStringObject), the object will be allocated, but too small. Then, 
memory gets clobbered.

If it returned NULL like it should, posix_read's error handling would be 
fine.
msg23512 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2004-12-04 04:29
Logged In: YES 
user_id=80475

In both Py2.3.4 and Py2.4, I get the following correct
behavior on WinME:

>>> os.read(si, -1)
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
OSError: [Errno 22] Invalid argument
msg23513 - (view) Author: George Yoshida (quiver) (Python committer) Date: 2004-12-04 11:43
Logged In: YES 
user_id=671362

On Win2k(Python 2.3.4 & 2.4), I get:
>>> os.read(si, -1)
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
OSError: [Errno 12] Not enough space

On Linux(SUSE 9.2 & kernel 2.6.5-7.108-smp & gcc 3.3.3) 
in Python 2.4 debug built, I get:

>>>os.read(si, -1)
asd
Debug memory block at address p=0x4024d6b8:
    31 bytes originally requested
    The 4 pad bytes at p-4 are FORBIDDENBYTE, as expected.
    The 4 pad bytes at tail=0x4024d6d7 are not all 
FORBIDDENBYTE
(0xfb):
        at tail+0: 0x0a *** OUCH
        at tail+1: 0xfb
        at tail+2: 0xfb
        at tail+3: 0xfb
    The block was made by call #10310 to debug 
malloc/realloc.
    Data at p: 00 00 00 00 00 00 00 00 ... ff 00 00 00 00 61 
73 64
Fatal Python error: bad trailing pad byte
Aborted

In a normal built, same as Jp.
msg23514 - (view) Author: Gerrit Holl (gerrit) Date: 2004-12-04 20:13
Logged In: YES 
user_id=13298

FWIW, another data point, Python 2.4., Linux 2.6.9, Fedora
Core 3:

$ python2.4 t.py < /usr/src/linux/README
Traceback (most recent call last):
  File "t.py", line 3, in ?
    os.read(0, -1)
OSError: [Errno 22] Invalid argument
$ python2.4 t.py < /dev/zero
Traceback (most recent call last):
  File "t.py", line 3, in ?
    os.read(0, -1)
OSError: [Errno 14] Bad address
$ python2.4 t.py < /dev/urandom
Segmentation fault

Interesting.
msg23515 - (view) Author: Michael Hudson (mwh) (Python committer) Date: 2004-12-04 20:38
Logged In: YES 
user_id=6656

I'm surprised at all this discussion.  It's a clear bug.  The only question is 
what the error message should be.  The attached makes it 

OSError: [Errno 22] Invalid argument

which seems most faithful to what the read() syscall does.
msg23516 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2004-12-04 20:58
Logged In: YES 
user_id=80475

No doubt it is a clear bug.  My note was just a data point.
 Had I been able to reproduce the error on my machine, I
would have been able to make a test_case and checkin a fix.

So, please, if you can demonstrate the error, go ahead and
check-in a fix with a testcase.

The OSError is probably fine though there is an alternative
of having a ValueError raised immediately after the args are
parsed in the read() method.

Also, you could prevent/detect future errors by adding an
assertion (checking for negative arguments) to
PyString_FromStringAndSize().
msg23517 - (view) Author: Michael Hudson (mwh) (Python committer) Date: 2004-12-04 21:11
Logged In: YES 
user_id=6656

Hmm.  Did you try a debug build and/or a range of arguments?

Is os.read actually tested anywhere?  I can't find any...

> Also, you could prevent/detect future errors by adding an
> assertion (checking for negative arguments) to
> PyString_FromStringAndSize().

Did you read the patch? <wink>
msg23518 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2004-12-05 00:39
Logged In: YES 
user_id=80475

No, I simply tried the OP's example and reported its
behavior on my system.

If you don't want to create a new test file, try adding this
on to test_subprocess.

Yes, I read the patch.  Yes, I forgot you added the
assertion already.

So are you going commit or wait for an engraved invitation?
 <wink>
msg23519 - (view) Author: Michael Hudson (mwh) (Python committer) Date: 2004-12-05 10:30
Logged In: YES 
user_id=6656

I'm waiting until I'm not behind a modem at my parents' house :)

Tomorrow.
msg23520 - (view) Author: Michael Hudson (mwh) (Python committer) Date: 2005-01-31 17:12
Logged In: YES 
user_id=6656

Finally fixed this (odd definition of "tomorrow", I know...)

Misc/NEWS revision 1.1236
Objects/stringobject.py revision 2.227
Modules/posixmodule.c revision 2.333

Sorry for the wait...
History
Date User Action Args
2022-04-11 14:56:08adminsetgithub: 41273
2004-12-01 21:40:50exarkuncreate