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.