[Python-checkins] bpo-33871: Fix os.sendfile(), os.writev(), os.readv(), etc. (GH-7931)
Miss Islington (bot)
webhook-mailer at python.org
Tue Jul 31 05:20:13 EDT 2018
https://github.com/python/cpython/commit/3e4b68875917a4605b45918f9e3232730fed9399
commit: 3e4b68875917a4605b45918f9e3232730fed9399
branch: 3.7
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: GitHub <noreply at github.com>
date: 2018-07-31T02:20:06-07:00
summary:
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.
(cherry picked from commit 9d5727326af53ddd91016d98e16ae7cf829caa95)
Co-authored-by: Serhiy Storchaka <storchaka at gmail.com>
files:
A Misc/NEWS.d/next/Library/2018-06-26-19-03-56.bpo-33871.XhlrGU.rst
A Misc/NEWS.d/next/Security/2018-06-26-19-35-33.bpo-33871.S4HR9n.rst
M Lib/test/test_os.py
M Lib/test/test_posix.py
M Modules/posixmodule.c
diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
index e509188243f6..a140ae0d29ea 100644
--- a/Lib/test/test_os.py
+++ b/Lib/test/test_os.py
@@ -2532,12 +2532,14 @@ class Handler(asynchat.async_chat):
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 @@ def tearDown(self):
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 @@ def test_keywords(self):
@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 @@ def test_trailers(self):
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'),
diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py
index 7dea1beab2c2..d2cb1c9840ea 100644
--- a/Lib/test/test_posix.py
+++ b/Lib/test/test_posix.py
@@ -20,6 +20,9 @@
_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 @@ def test_preadv(self):
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 @@ def test_preadv_flags(self):
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 @@ def test_pwritev(self):
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 @@ def test_pwritev_flags(self):
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 @@ def test_writev(self):
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 @@ def test_readv(self):
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):
diff --git a/Misc/NEWS.d/next/Library/2018-06-26-19-03-56.bpo-33871.XhlrGU.rst b/Misc/NEWS.d/next/Library/2018-06-26-19-03-56.bpo-33871.XhlrGU.rst
new file mode 100644
index 000000000000..9fd15356c72d
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2018-06-26-19-03-56.bpo-33871.XhlrGU.rst
@@ -0,0 +1,3 @@
+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).
diff --git a/Misc/NEWS.d/next/Security/2018-06-26-19-35-33.bpo-33871.S4HR9n.rst b/Misc/NEWS.d/next/Security/2018-06-26-19-35-33.bpo-33871.S4HR9n.rst
new file mode 100644
index 000000000000..547342c2e9dd
--- /dev/null
+++ b/Misc/NEWS.d/next/Security/2018-06-26-19-35-33.bpo-33871.S4HR9n.rst
@@ -0,0 +1,3 @@
+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.
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index c3682b4a0fd5..d487e5386e89 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -8026,12 +8026,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) {
@@ -8056,11 +8057,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);
@@ -8357,12 +8356,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
}
}
@@ -8383,13 +8390,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
}
}
}
More information about the Python-checkins
mailing list