[Twisted-Python] twisted.python.sendmsg segfault - looking for confirmation
Dear all, While working on http://twistedmatrix.com/trac/ticket/8912 and the associated PR at https://github.com/twisted/twisted/pull/647 I hit a segmentation fault on a test I was preparing. I think narrowed it down Linux + Python 2, but I'm not 100% sure. Can anyone please confirm the code below segfaults on such environment? For completeness, from my tests, it works fine on Linux + Python 3, and fails with socket.error/OSError on Mac OS 10.9.5 + Python 2/3. No BSDs at hand to try out... As far as I can tell, no code in Twisted uses sendmsg in the way the code is using it. However, twisted.python.sendmsg is a public API so someone somewhere may hit this like I just did. Other than looking for confirmation, I have a fix which I included two isolated commits in the PR. Twisted project members: if this is confirmed, maybe a ticket should be created in Trac and an independent PR created, correct? Thanks in advance for any input. Regards, -- exvito -[ cut here ]---------------------------------------------------------------------- import os import socket import struct import sys from twisted.python import sendmsg if __name__ == '__main__': # This code segfaults when running with twisted/trunk commit # 98a3df200968b78bd3b985dfd4fb10a5b415d6fc on Linux Python 2 # due to a bug in src/twisted/python/_sendmsg.c and glibc's # CMSG_NXTHDR implementation. # get connected sockets s1, s2 = socket.socketpair(socket.AF_UNIX) # two CMSGs in ancillary data (no segfault with just one) ancillary = [ (socket.SOL_SOCKET, sendmsg.SCM_RIGHTS, struct.pack('i', s1.fileno())), (socket.SOL_SOCKET, sendmsg.SCM_RIGHTS, struct.pack('i', s2.fileno())), ] expected = { 2: 'Expecting to segfault.', 3: 'Should not segfault.', } print expected.get(sys.version_info.major, 'Unexpected Python version.') retval = sendmsg.sendmsg(s1, data=b'some data', ancillary=ancillary) print 'Did not segfault.' -[ cut here ]----------------------------------------------------------------------
On 28 Dec. 2016, at 14:15, ex vito <ex.vitorino@gmail.com> wrote:
Dear all,
While working on http://twistedmatrix.com/trac/ticket/8912 and the associated PR at https://github.com/twisted/twisted/pull/647 I hit a segmentation fault on a test I was preparing.
I think narrowed it down Linux + Python 2, but I'm not 100% sure. Can anyone please confirm the code below segfaults on such environment? For completeness, from my tests, it works fine on Linux + Python 3, and fails with socket.error/OSError on Mac OS 10.9.5 + Python 2/3. No BSDs at hand to try out...
As far as I can tell, no code in Twisted uses sendmsg in the way the code is using it. However, twisted.python.sendmsg is a public API so someone somewhere may hit this like I just did.
Other than looking for confirmation, I have a fix which I included two isolated commits in the PR. Twisted project members: if this is confirmed, maybe a ticket should be created in Trac and an independent PR created, correct?
Thanks in advance for any input. Regards, -- exvito
-[ cut here ]----------------------------------------------------------------------
import os import socket import struct import sys
from twisted.python import sendmsg
if __name__ == '__main__':
# This code segfaults when running with twisted/trunk commit # 98a3df200968b78bd3b985dfd4fb10a5b415d6fc on Linux Python 2 # due to a bug in src/twisted/python/_sendmsg.c and glibc's # CMSG_NXTHDR implementation.
# get connected sockets s1, s2 = socket.socketpair(socket.AF_UNIX)
# two CMSGs in ancillary data (no segfault with just one) ancillary = [ (socket.SOL_SOCKET, sendmsg.SCM_RIGHTS, struct.pack('i', s1.fileno())), (socket.SOL_SOCKET, sendmsg.SCM_RIGHTS, struct.pack('i', s2.fileno())), ]
expected = { 2: 'Expecting to segfault.', 3: 'Should not segfault.', } print expected.get(sys.version_info.major, 'Unexpected Python version.') retval = sendmsg.sendmsg(s1, data=b'some data', ancillary=ancillary) print 'Did not segfault.'
-[ cut here ]----------------------------------------------------------------------
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
FWIW, for people wanting to track this down, this may be a bug in our C extension (src/twisted/python/_sendmsg.c). Python 3 uses the stdlib's functionality, which is probably why it isn't running into this. - Amber
On Dec 27, 2016, at 19:15, ex vito <ex.vitorino@gmail.com> wrote:
As far as I can tell, no code in Twisted uses sendmsg in the way the code is using it. However, twisted.python.sendmsg is a public API so someone somewhere may hit this like I just did.
Quite.
Other than looking for confirmation, I have a fix which I included two isolated commits in the PR. Twisted project members: if this is confirmed, maybe a ticket should be created in Trac and an independent PR created, correct?
You should definitely create that ticket. In fact, you don't need to wait for confirmation - create the ticket now, and send the link to it, so someone else can comment on the ticket when it is confirmed - discussion of the reproduction process can happen there. -g
On 2016-12-29, at 12:23, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Dec 27, 2016, at 19:15, ex vito <ex.vitorino@gmail.com> wrote:
Other than looking for confirmation, I have a fix which I included two isolated commits in the PR. Twisted project members: if this is confirmed, maybe a ticket should be created in Trac and an independent PR created, correct?
You should definitely create that ticket. In fact, you don't need to wait for confirmation - create the ticket now, and send the link to it, so someone else can comment on the ticket when it is confirmed - discussion of the reproduction process can happen there.
Done, Ticket: https://twistedmatrix.com/trac/ticket/8969 PR: https://github.com/twisted/twisted/pull/650 Up for review with a guidance request on how to achieve 100% diff coverage given that different platforms behave differently with the same input. Thanks and regards, -- exvito
On Dec 29, 2016, at 11:01 AM, ex vito <ex.vitorino@gmail.com> wrote:
On 2016-12-29, at 12:23, Glyph Lefkowitz <glyph@twistedmatrix.com <mailto:glyph@twistedmatrix.com>> wrote:
On Dec 27, 2016, at 19:15, ex vito <ex.vitorino@gmail.com <mailto:ex.vitorino@gmail.com>> wrote:
Other than looking for confirmation, I have a fix which I included two isolated commits in the PR. Twisted project members: if this is confirmed, maybe a ticket should be created in Trac and an independent PR created, correct?
You should definitely create that ticket. In fact, you don't need to wait for confirmation - create the ticket now, and send the link to it, so someone else can comment on the ticket when it is confirmed - discussion of the reproduction process can happen there.
Done,
Ticket: https://twistedmatrix.com/trac/ticket/8969 <https://twistedmatrix.com/trac/ticket/8969> PR: https://github.com/twisted/twisted/pull/650 <https://github.com/twisted/twisted/pull/650>
Up for review with a guidance request on how to achieve 100% diff coverage given that different platforms behave differently with the same input.
All our supported platforms submit coverage reports; the coverage reported by codecov should be the union of all those reports. So, when your code gets pushed to a branch and run on the buildbots, the coverage report should indicate that. -glyph
On 30 Dec 2016, at 01:56, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Dec 29, 2016, at 11:01 AM, ex vito <ex.vitorino@gmail.com> wrote:
On 2016-12-29, at 12:23, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Dec 27, 2016, at 19:15, ex vito <ex.vitorino@gmail.com> wrote:
Other than looking for confirmation, I have a fix which I included two isolated commits in the PR. Twisted project members: if this is confirmed, maybe a ticket should be created in Trac and an independent PR created, correct?
You should definitely create that ticket. In fact, you don't need to wait for confirmation - create the ticket now, and send the link to it, so someone else can comment on the ticket when it is confirmed - discussion of the reproduction process can happen there.
Done,
Ticket: https://twistedmatrix.com/trac/ticket/8969 PR: https://github.com/twisted/twisted/pull/650
Up for review with a guidance request on how to achieve 100% diff coverage given that different platforms behave differently with the same input.
All our supported platforms submit coverage reports; the coverage reported by codecov should be the union of all those reports. So, when your code gets pushed to a branch and run on the buildbots, the coverage report should indicate that.
Oh, nice! I hadn't figured that out, yet. It makes sense and after rodrigc pushed it to the buildbots it did confirm total diff coverage. Thanks to both. Regards, -- exvito
participants (4)
-
Amber "Hawkie" Brown -
ex vito -
Ex Vitorino
-
Glyph Lefkowitz