Bugs item #1433877, was opened at 2006-02-17 17:29
Message generated for change (Comment added) made by gvanrossum
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1433877&group_id=5470
Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Extension Modules
Group: Python 2.4
Status: Open
Resolution: None
Priority: 9
Submitted By: Quentin Barnes (qbarnes)
Assigned to: Thomas Wouters (twouters)
Summary: string parameter to ioctl not null terminated, includes fix
Initial Comment:
I ran across this problem in Python 2.3.3 and see it is
still there in 2.4.2.
When running the test_pty.py test case, I would get
intermittant failures depending on how the test was
invoked or the compiler flags used on Solaris.
Trussing the interpreter while running the test revealed:
ioctl(4, I_PUSH, "ptem\xff^T^F^H") Err#22 EINVAL
There was garbage on the end of the string when it
would fail.
I tracked the problem back to fcntl_ioctl() in
fcntlmodule.c. The string was not being NULL
terminated, but relied on having zeroed gargage on the
stack to work.
I checked the source for 2.4.2 and noticed the same
problem. Patch to fix the problem is attached.
----------------------------------------------------------------------
>Comment By: Guido van Rossum (gvanrossum)
Date: 2006-02-19 15:03
Message:
Logged In: YES
user_id=6380
One problem with ioctl() is that the required length of the
buffer is unknown unless you parse the operation code
argument in a very platform- (and probably even kernel- and
driver-) dependent way. Whether null-terminating it makes
sense or not depends on the particular operation code.
I don't think the patch makes ioctl "safe", and I'm not sure
it even ought to be applied. A quick code review reveals a
few more issues:
- the docstring doesn't explain the optional "mutate_flag"
argument (which BTW is a mess -- read the online docs -- why
are we changing the default to true?)
- the online docs ought to be updated for 2.4 and again for
2.5 -- they still speak of 2.4 in the future tense! Also,
it's a bit strong to say this function is "identical" to
fcntl() -- "similar" sounds more like it.
- In the first branch of the function, this line:
if (mutate_arg && (len < sizeof buf)) {
ought to test (len <= sizeof buf) to match the test earlier;
but probably better is to use
if (mutate_arg && arg == buf) {
- If it's important to null-terminate the data in the buffer
when a string is passed in, shouldn't we null-terminate it
also when a writable buffer-interface object is passed in?
If not in the latter case, why if a string is passed? One
could argue that a string with an explicit \0 (visible to
Python code) at the end ought to be passed in if the
semantics of the op code requires a null-terminated argument
(which most ioctl op codes don't -- the typical argument is
a C struct).
- The proposed patch reduces the maximum buffer size to
1023, which violates the docs. (Yes the docs explicitly
mention 1024.)
- The best we can do seems to be: increase the buffer size
to 1025; null-pad it in all cases; change the code for
mutable buffers to test for sizeof buf - 1. This still
leaves the case where the buffer isn't used because a
mutable buffer is passed that's longer than 1024. Which
suggests that we can't completely fix this -- it will always
be possible to crash Python using this function by passing
an op code that encodes a buffer length > 1024 while passing
a shorter argument, so the internal buffer is used. I guess
we could do the null-padding as long as we document it and
document that when a mutable buffer is passed we don't
guarantee it.
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2006-02-18 02:14
Message:
Logged In: YES
user_id=33168
Thomas, could this have caused the flakiness that you just
fixed with test_pty? This patch seems correct to me and I
think it should be applied. (If you want I can do that, but
I wanted you to see this first.)
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1433877&group_id=5470
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
