Skip to content

Commit 18bf8f8

Browse files
gh-95380: Remove the 1024 bytes limit in fcntl.fcntl() and fcntl.ioctl() (GH-132907)
1 parent fe9f6e8 commit 18bf8f8

File tree

5 files changed

+147
-46
lines changed

5 files changed

+147
-46
lines changed

Doc/library/fcntl.rst

+12-6
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,15 @@ The module defines the following functions:
107107
passed to the C :c:func:`fcntl` call. The return value after a successful
108108
call is the contents of the buffer, converted to a :class:`bytes` object.
109109
The length of the returned object will be the same as the length of the
110-
*arg* argument. This is limited to 1024 bytes.
110+
*arg* argument.
111111

112112
If the :c:func:`fcntl` call fails, an :exc:`OSError` is raised.
113113

114114
.. note::
115-
If the type or the size of *arg* does not match the type or size
116-
of the argument of the operation (for example, if an integer is
115+
If the type or size of *arg* does not match the type or size
116+
of the operation's argument (for example, if an integer is
117117
passed when a pointer is expected, or the information returned in
118-
the buffer by the operating system is larger than 1024 bytes),
118+
the buffer by the operating system is larger than the size of *arg*),
119119
this is most likely to result in a segmentation violation or
120120
a more subtle data corruption.
121121

@@ -125,6 +125,9 @@ The module defines the following functions:
125125
Add support of arbitrary :term:`bytes-like objects <bytes-like object>`,
126126
not only :class:`bytes`.
127127

128+
.. versionchanged:: next
129+
The size of bytes-like objects is no longer limited to 1024 bytes.
130+
128131

129132
.. function:: ioctl(fd, request, arg=0, mutate_flag=True, /)
130133

@@ -161,8 +164,7 @@ The module defines the following functions:
161164
If the type or size of *arg* does not match the type or size
162165
of the operation's argument (for example, if an integer is
163166
passed when a pointer is expected, or the information returned in
164-
the buffer by the operating system is larger than 1024 bytes,
165-
or the size of the mutable bytes-like object is too small),
167+
the buffer by the operating system is larger than the size of *arg*),
166168
this is most likely to result in a segmentation violation or
167169
a more subtle data corruption.
168170

@@ -185,6 +187,10 @@ The module defines the following functions:
185187
The GIL is always released during a system call.
186188
System calls failing with EINTR are automatically retried.
187189

190+
.. versionchanged:: next
191+
The size of not mutated bytes-like objects is no longer
192+
limited to 1024 bytes.
193+
188194
.. function:: flock(fd, operation, /)
189195

190196
Perform the lock operation *operation* on file descriptor *fd* (file objects providing

Lib/test/test_fcntl.py

+46
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,52 @@ def test_fcntl_f_pipesize(self):
228228
os.close(test_pipe_r)
229229
os.close(test_pipe_w)
230230

231+
def _check_fcntl_not_mutate_len(self, nbytes=None):
232+
self.f = open(TESTFN, 'wb')
233+
buf = struct.pack('ii', fcntl.F_OWNER_PID, os.getpid())
234+
if nbytes is not None:
235+
buf += b' ' * (nbytes - len(buf))
236+
else:
237+
nbytes = len(buf)
238+
save_buf = bytes(buf)
239+
r = fcntl.fcntl(self.f, fcntl.F_SETOWN_EX, buf)
240+
self.assertIsInstance(r, bytes)
241+
self.assertEqual(len(r), len(save_buf))
242+
self.assertEqual(buf, save_buf)
243+
type, pid = memoryview(r).cast('i')[:2]
244+
self.assertEqual(type, fcntl.F_OWNER_PID)
245+
self.assertEqual(pid, os.getpid())
246+
247+
buf = b' ' * nbytes
248+
r = fcntl.fcntl(self.f, fcntl.F_GETOWN_EX, buf)
249+
self.assertIsInstance(r, bytes)
250+
self.assertEqual(len(r), len(save_buf))
251+
self.assertEqual(buf, b' ' * nbytes)
252+
type, pid = memoryview(r).cast('i')[:2]
253+
self.assertEqual(type, fcntl.F_OWNER_PID)
254+
self.assertEqual(pid, os.getpid())
255+
256+
buf = memoryview(b' ' * nbytes)
257+
r = fcntl.fcntl(self.f, fcntl.F_GETOWN_EX, buf)
258+
self.assertIsInstance(r, bytes)
259+
self.assertEqual(len(r), len(save_buf))
260+
self.assertEqual(bytes(buf), b' ' * nbytes)
261+
type, pid = memoryview(r).cast('i')[:2]
262+
self.assertEqual(type, fcntl.F_OWNER_PID)
263+
self.assertEqual(pid, os.getpid())
264+
265+
@unittest.skipUnless(
266+
hasattr(fcntl, "F_SETOWN_EX") and hasattr(fcntl, "F_GETOWN_EX"),
267+
"requires F_SETOWN_EX and F_GETOWN_EX")
268+
def test_fcntl_small_buffer(self):
269+
self._check_fcntl_not_mutate_len()
270+
271+
@unittest.skipUnless(
272+
hasattr(fcntl, "F_SETOWN_EX") and hasattr(fcntl, "F_GETOWN_EX"),
273+
"requires F_SETOWN_EX and F_GETOWN_EX")
274+
def test_fcntl_large_buffer(self):
275+
self._check_fcntl_not_mutate_len(2024)
276+
231277

232278
if __name__ == '__main__':
233279
unittest.main()

Lib/test/test_ioctl.py

+1-2
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,8 @@ def test_ioctl_mutate_1024(self):
127127
self._check_ioctl_not_mutate_len(1024)
128128

129129
def test_ioctl_mutate_2048(self):
130-
# Test with a larger buffer, just for the record.
131130
self._check_ioctl_mutate_len(2048)
132-
self.assertRaises(ValueError, self._check_ioctl_not_mutate_len, 2048)
131+
self._check_ioctl_not_mutate_len(1024)
133132

134133

135134
@unittest.skipUnless(hasattr(os, 'openpty'), "need os.openpty()")
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
:func:`fcntl.fcntl` and :func:`fcntl.ioctl`: Remove the 1024 bytes limit
2+
on the size of not mutated bytes-like argument.

Modules/fcntlmodule.c

+86-38
Original file line numberDiff line numberDiff line change
@@ -93,29 +93,53 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg)
9393
return NULL;
9494
}
9595
Py_ssize_t len = view.len;
96-
if (len > FCNTL_BUFSZ) {
97-
PyErr_SetString(PyExc_ValueError,
98-
"fcntl argument 3 is too long");
96+
if (len <= FCNTL_BUFSZ) {
97+
memcpy(buf, view.buf, len);
98+
memcpy(buf + len, guard, GUARDSZ);
9999
PyBuffer_Release(&view);
100-
return NULL;
101-
}
102-
memcpy(buf, view.buf, len);
103-
memcpy(buf + len, guard, GUARDSZ);
104-
PyBuffer_Release(&view);
105100

106-
do {
107-
Py_BEGIN_ALLOW_THREADS
108-
ret = fcntl(fd, code, buf);
109-
Py_END_ALLOW_THREADS
110-
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
111-
if (ret < 0) {
112-
return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL;
101+
do {
102+
Py_BEGIN_ALLOW_THREADS
103+
ret = fcntl(fd, code, buf);
104+
Py_END_ALLOW_THREADS
105+
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
106+
if (ret < 0) {
107+
return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL;
108+
}
109+
if (memcmp(buf + len, guard, GUARDSZ) != 0) {
110+
PyErr_SetString(PyExc_SystemError, "buffer overflow");
111+
return NULL;
112+
}
113+
return PyBytes_FromStringAndSize(buf, len);
113114
}
114-
if (memcmp(buf + len, guard, GUARDSZ) != 0) {
115-
PyErr_SetString(PyExc_SystemError, "buffer overflow");
116-
return NULL;
115+
else {
116+
PyObject *result = PyBytes_FromStringAndSize(NULL, len);
117+
if (result == NULL) {
118+
PyBuffer_Release(&view);
119+
return NULL;
120+
}
121+
char *ptr = PyBytes_AsString(result);
122+
memcpy(ptr, view.buf, len);
123+
PyBuffer_Release(&view);
124+
125+
do {
126+
Py_BEGIN_ALLOW_THREADS
127+
ret = fcntl(fd, code, ptr);
128+
Py_END_ALLOW_THREADS
129+
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
130+
if (ret < 0) {
131+
if (async_err) {
132+
PyErr_SetFromErrno(PyExc_OSError);
133+
}
134+
Py_DECREF(result);
135+
return NULL;
136+
}
137+
if (ptr[len] != '\0') {
138+
PyErr_SetString(PyExc_SystemError, "buffer overflow");
139+
return NULL;
140+
}
141+
return result;
117142
}
118-
return PyBytes_FromStringAndSize(buf, len);
119143
#undef FCNTL_BUFSZ
120144
}
121145
PyErr_Format(PyExc_TypeError,
@@ -251,29 +275,53 @@ fcntl_ioctl_impl(PyObject *module, int fd, unsigned long code, PyObject *arg,
251275
return NULL;
252276
}
253277
Py_ssize_t len = view.len;
254-
if (len > IOCTL_BUFSZ) {
255-
PyErr_SetString(PyExc_ValueError,
256-
"ioctl argument 3 is too long");
278+
if (len <= IOCTL_BUFSZ) {
279+
memcpy(buf, view.buf, len);
280+
memcpy(buf + len, guard, GUARDSZ);
257281
PyBuffer_Release(&view);
258-
return NULL;
259-
}
260-
memcpy(buf, view.buf, len);
261-
memcpy(buf + len, guard, GUARDSZ);
262-
PyBuffer_Release(&view);
263282

264-
do {
265-
Py_BEGIN_ALLOW_THREADS
266-
ret = ioctl(fd, code, buf);
267-
Py_END_ALLOW_THREADS
268-
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
269-
if (ret < 0) {
270-
return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL;
283+
do {
284+
Py_BEGIN_ALLOW_THREADS
285+
ret = ioctl(fd, code, buf);
286+
Py_END_ALLOW_THREADS
287+
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
288+
if (ret < 0) {
289+
return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL;
290+
}
291+
if (memcmp(buf + len, guard, GUARDSZ) != 0) {
292+
PyErr_SetString(PyExc_SystemError, "buffer overflow");
293+
return NULL;
294+
}
295+
return PyBytes_FromStringAndSize(buf, len);
271296
}
272-
if (memcmp(buf + len, guard, GUARDSZ) != 0) {
273-
PyErr_SetString(PyExc_SystemError, "buffer overflow");
274-
return NULL;
297+
else {
298+
PyObject *result = PyBytes_FromStringAndSize(NULL, len);
299+
if (result == NULL) {
300+
PyBuffer_Release(&view);
301+
return NULL;
302+
}
303+
char *ptr = PyBytes_AsString(result);
304+
memcpy(ptr, view.buf, len);
305+
PyBuffer_Release(&view);
306+
307+
do {
308+
Py_BEGIN_ALLOW_THREADS
309+
ret = ioctl(fd, code, ptr);
310+
Py_END_ALLOW_THREADS
311+
} while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
312+
if (ret < 0) {
313+
if (async_err) {
314+
PyErr_SetFromErrno(PyExc_OSError);
315+
}
316+
Py_DECREF(result);
317+
return NULL;
318+
}
319+
if (ptr[len] != '\0') {
320+
PyErr_SetString(PyExc_SystemError, "buffer overflow");
321+
return NULL;
322+
}
323+
return result;
275324
}
276-
return PyBytes_FromStringAndSize(buf, len);
277325
#undef IOCTL_BUFSZ
278326
}
279327
PyErr_Format(PyExc_TypeError,

0 commit comments

Comments
 (0)