Skip to content

C
cpython
Commit 9d572732 authored by Serhiy Storchaka's avatar Serhiy Storchaka Committed by GitHub
Browse files
Options

bpo-33871: Fix os.sendfile(), os.writev(), os.readv(), etc. (GH-7931) 

* Fix integer overflow in os.readv(), os.writev(), os.preadv()
  and os.pwritev() and in os.sendfile() with headers or trailers
  arguments (on BSD-based OSes and MacOS).

* Fix sending the part of the file in os.sendfile() on MacOS.
  Using the trailers argument could cause sending more bytes from
  the input file than was specified.

Thanks Ned Deily for testing on 32-bit MacOS.
parent f1d36d8e
Hide whitespace changes
Inline Side-by-side
Showing with 114 additions and 32 deletions
Lib/test/test_os.py
View file @ 9d572732
......@@ -2532,12 +2532,14 @@ class SendfileTestServer(asyncore.dispatcher, threading.Thread):
def __init__(self, conn):
asynchat.async_chat.__init__(self, conn)
self.in_buffer = []
self.accumulate = True
self.closed = False
self.push(b"220 ready\r\n")
def handle_read(self):
data = self.recv(4096)
self.in_buffer.append(data)
if self.accumulate:
self.in_buffer.append(data)
def get_data(self):
return b''.join(self.in_buffer)
......@@ -2618,6 +2620,8 @@ class TestSendfile(unittest.TestCase):
not sys.platform.startswith("sunos")
requires_headers_trailers = unittest.skipUnless(SUPPORT_HEADERS_TRAILERS,
'requires headers and trailers support')
requires_32b = unittest.skipUnless(sys.maxsize < 2**32,
'test is only meaningful on 32-bit builds')
@classmethod
def setUpClass(cls):
......@@ -2648,17 +2652,13 @@ class TestSendfile(unittest.TestCase):
self.server.stop()
self.server = None
def sendfile_wrapper(self, sock, file, offset, nbytes, headers=[], trailers=[]):
def sendfile_wrapper(self, *args, **kwargs):
"""A higher level wrapper representing how an application is
supposed to use sendfile().
"""
while 1:
while True:
try:
if self.SUPPORT_HEADERS_TRAILERS:
return os.sendfile(sock, file, offset, nbytes, headers,
trailers)
else:
return os.sendfile(sock, file, offset, nbytes)
return os.sendfile(*args, **kwargs)
except OSError as err:
if err.errno == errno.ECONNRESET:
# disconnected
......@@ -2749,20 +2749,22 @@ class TestSendfile(unittest.TestCase):
@requires_headers_trailers
def test_headers(self):
total_sent = 0
expected_data = b"x" * 512 + b"y" * 256 + self.DATA[:-1]
sent = os.sendfile(self.sockno, self.fileno, 0, 4096,
headers=[b"x" * 512])
headers=[b"x" * 512, b"y" * 256])
self.assertLessEqual(sent, 512 + 256 + 4096)
total_sent += sent
offset = 4096
nbytes = 4096
while 1:
while total_sent < len(expected_data):
nbytes = min(len(expected_data) - total_sent, 4096)
sent = self.sendfile_wrapper(self.sockno, self.fileno,
offset, nbytes)
if sent == 0:
break
self.assertLessEqual(sent, nbytes)
total_sent += sent
offset += sent
expected_data = b"x" * 512 + self.DATA
self.assertEqual(total_sent, len(expected_data))
self.client.close()
self.server.wait()
......@@ -2778,12 +2780,30 @@ class TestSendfile(unittest.TestCase):
create_file(TESTFN2, file_data)
with open(TESTFN2, 'rb') as f:
os.sendfile(self.sockno, f.fileno(), 0, len(file_data),
trailers=[b"1234"])
os.sendfile(self.sockno, f.fileno(), 0, 5,
trailers=[b"123456", b"789"])
self.client.close()
self.server.wait()
data = self.server.handler_instance.get_data()
self.assertEqual(data, b"abcdef1234")
self.assertEqual(data, b"abcde123456789")
@requires_headers_trailers
@requires_32b
def test_headers_overflow_32bits(self):
self.server.handler_instance.accumulate = False
with self.assertRaises(OSError) as cm:
os.sendfile(self.sockno, self.fileno, 0, 0,
headers=[b"x" * 2**16] * 2**15)
self.assertEqual(cm.exception.errno, errno.EINVAL)
@requires_headers_trailers
@requires_32b
def test_trailers_overflow_32bits(self):
self.server.handler_instance.accumulate = False
with self.assertRaises(OSError) as cm:
os.sendfile(self.sockno, self.fileno, 0, 0,
trailers=[b"x" * 2**16] * 2**15)
self.assertEqual(cm.exception.errno, errno.EINVAL)
@requires_headers_trailers
@unittest.skipUnless(hasattr(os, 'SF_NODISKIO'),
......
Lib/test/test_posix.py
View file @ 9d572732
......@@ -20,6 +20,9 @@ import warnings
_DUMMY_SYMLINK = os.path.join(tempfile.gettempdir(),
support.TESTFN + '-dummy-symlink')
requires_32b = unittest.skipUnless(sys.maxsize < 2**32,
'test is only meaningful on 32-bit builds')
class PosixTester(unittest.TestCase):
def setUp(self):
......@@ -284,6 +287,7 @@ class PosixTester(unittest.TestCase):
finally:
os.close(fd)
@unittest.skipUnless(hasattr(posix, 'preadv'), "test needs posix.preadv()")
@unittest.skipUnless(hasattr(posix, 'RWF_HIPRI'), "test needs posix.RWF_HIPRI")
def test_preadv_flags(self):
fd = os.open(support.TESTFN, os.O_RDWR | os.O_CREAT)
......@@ -295,6 +299,19 @@ class PosixTester(unittest.TestCase):
finally:
os.close(fd)
@unittest.skipUnless(hasattr(posix, 'preadv'), "test needs posix.preadv()")
@requires_32b
def test_preadv_overflow_32bits(self):
fd = os.open(support.TESTFN, os.O_RDWR | os.O_CREAT)
try:
buf = [bytearray(2**16)] * 2**15
with self.assertRaises(OSError) as cm:
os.preadv(fd, buf, 0)
self.assertEqual(cm.exception.errno, errno.EINVAL)
self.assertEqual(bytes(buf[0]), b'\0'* 2**16)
finally:
os.close(fd)
@unittest.skipUnless(hasattr(posix, 'pwrite'), "test needs posix.pwrite()")
def test_pwrite(self):
fd = os.open(support.TESTFN, os.O_RDWR | os.O_CREAT)
......@@ -320,6 +337,7 @@ class PosixTester(unittest.TestCase):
finally:
os.close(fd)
@unittest.skipUnless(hasattr(posix, 'pwritev'), "test needs posix.pwritev()")
@unittest.skipUnless(hasattr(posix, 'os.RWF_SYNC'), "test needs os.RWF_SYNC")
def test_pwritev_flags(self):
fd = os.open(support.TESTFN, os.O_RDWR | os.O_CREAT)
......@@ -334,6 +352,17 @@ class PosixTester(unittest.TestCase):
finally:
os.close(fd)
@unittest.skipUnless(hasattr(posix, 'pwritev'), "test needs posix.pwritev()")
@requires_32b
def test_pwritev_overflow_32bits(self):
fd = os.open(support.TESTFN, os.O_RDWR | os.O_CREAT)
try:
with self.assertRaises(OSError) as cm:
os.pwritev(fd, [b"x" * 2**16] * 2**15, 0)
self.assertEqual(cm.exception.errno, errno.EINVAL)
finally:
os.close(fd)
@unittest.skipUnless(hasattr(posix, 'posix_fallocate'),
"test needs posix.posix_fallocate()")
def test_posix_fallocate(self):
......@@ -435,6 +464,17 @@ class PosixTester(unittest.TestCase):
finally:
os.close(fd)
@unittest.skipUnless(hasattr(posix, 'writev'), "test needs posix.writev()")
@requires_32b
def test_writev_overflow_32bits(self):
fd = os.open(support.TESTFN, os.O_RDWR | os.O_CREAT)
try:
with self.assertRaises(OSError) as cm:
os.writev(fd, [b"x" * 2**16] * 2**15)
self.assertEqual(cm.exception.errno, errno.EINVAL)
finally:
os.close(fd)
@unittest.skipUnless(hasattr(posix, 'readv'), "test needs posix.readv()")
def test_readv(self):
fd = os.open(support.TESTFN, os.O_RDWR | os.O_CREAT)
......@@ -457,6 +497,19 @@ class PosixTester(unittest.TestCase):
finally:
os.close(fd)
@unittest.skipUnless(hasattr(posix, 'readv'), "test needs posix.readv()")
@requires_32b
def test_readv_overflow_32bits(self):
fd = os.open(support.TESTFN, os.O_RDWR | os.O_CREAT)
try:
buf = [bytearray(2**16)] * 2**15
with self.assertRaises(OSError) as cm:
os.readv(fd, buf)
self.assertEqual(cm.exception.errno, errno.EINVAL)
self.assertEqual(bytes(buf[0]), b'\0'* 2**16)
finally:
os.close(fd)
@unittest.skipUnless(hasattr(posix, 'dup'),
'test needs posix.dup()')
def test_dup(self):
......
Misc/NEWS.d/next/Library/2018-06-26-19-03-56.bpo-33871.XhlrGU.rst 0 → 100644
View file @ 9d572732
Fixed integer overflow in :func:`os.readv`, :func:`os.writev`,
:func:`os.preadv` and :func:`os.pwritev` and in :func:`os.sendfile` with
*headers* or *trailers* arguments (on BSD-based OSes and macOS).
Misc/NEWS.d/next/Security/2018-06-26-19-35-33.bpo-33871.S4HR9n.rst 0 → 100644
View file @ 9d572732
Fixed sending the part of the file in :func:`os.sendfile` on macOS. Using
the *trailers* argument could cause sending more bytes from the input file
than was specified.
Modules/posixmodule.c
View file @ 9d572732
......@@ -8321,12 +8321,13 @@ os_read_impl(PyObject *module, int fd, Py_ssize_t length)
}
#if (defined(HAVE_SENDFILE) && (defined(__FreeBSD__) || defined(__DragonFly__) \
|| defined(__APPLE__))) || defined(HAVE_READV) || defined(HAVE_WRITEV)
static Py_ssize_t
|| defined(__APPLE__))) \
|| defined(HAVE_READV) || defined(HAVE_PREADV) || defined (HAVE_PREADV2) \
|| defined(HAVE_WRITEV) || defined(HAVE_PWRITEV) || defined (HAVE_PWRITEV2)
static int
iov_setup(struct iovec **iov, Py_buffer **buf, PyObject *seq, Py_ssize_t cnt, int type)
{
Py_ssize_t i, j;
Py_ssize_t blen, total = 0;
*iov = PyMem_New(struct iovec, cnt);
if (*iov == NULL) {
......@@ -8351,11 +8352,9 @@ iov_setup(struct iovec **iov, Py_buffer **buf, PyObject *seq, Py_ssize_t cnt, in
}
Py_DECREF(item);
(*iov)[i].iov_base = (*buf)[i].buf;
blen = (*buf)[i].len;
(*iov)[i].iov_len = blen;
total += blen;
(*iov)[i].iov_len = (*buf)[i].len;
}
return total;
return 0;
fail:
PyMem_Del(*iov);
......@@ -8652,12 +8651,20 @@ posix_sendfile(PyObject *self, PyObject *args, PyObject *kwdict)
}
if (i > 0) {
sf.hdr_cnt = (int)i;
i = iov_setup(&(sf.headers), &hbuf,
headers, sf.hdr_cnt, PyBUF_SIMPLE);
if (i < 0)
if (iov_setup(&(sf.headers), &hbuf,
headers, sf.hdr_cnt, PyBUF_SIMPLE) < 0)
return NULL;
#ifdef __APPLE__
sbytes += i;
for (i = 0; i < sf.hdr_cnt; i++) {
Py_ssize_t blen = sf.headers[i].iov_len;
# define OFF_T_MAX 0x7fffffffffffffff
if (sbytes >= OFF_T_MAX - blen) {
PyErr_SetString(PyExc_OverflowError,
"sendfile() header is too large");
return NULL;
}
sbytes += blen;
}
#endif
}
}
......@@ -8678,13 +8685,9 @@ posix_sendfile(PyObject *self, PyObject *args, PyObject *kwdict)
}
if (i > 0) {
sf.trl_cnt = (int)i;
i = iov_setup(&(sf.trailers), &tbuf,
trailers, sf.trl_cnt, PyBUF_SIMPLE);
if (i < 0)
if (iov_setup(&(sf.trailers), &tbuf,
trailers, sf.trl_cnt, PyBUF_SIMPLE) < 0)
return NULL;
#ifdef __APPLE__
sbytes += i;
#endif
}
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7