Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#5444 closed Bug (fixed)

underflow in libtransmission/bitfield.c's set_all_true()

Reported by: jordan Owned by: jordan
Priority: Normal Milestone: 2.82
Component: libtransmission Version: 2.80
Severity: Major Keywords:
Cc:

Description

Downstream ticket https://bugzilla.redhat.com/show_bug.cgi?id=989258 has this backtrace:

Thread 1 (Thread 0x7f2f777fe700 (LWP 5309)):
#0  __memset_sse2 () at ../sysdeps/x86_64/memset.S:907
No locals.
#1  0x00007f2f9334f11f in memset (__len=18446744073709551615, __ch=255, __dest=0x0) at /usr/include/bits/string3.h:84
No locals.
#2  set_all_true (array=array@entry=0x0, bit_count=0) at bitfield.c:182
        n = 0
#3  0x00007f2f9334f439 in tr_bitfieldGetRaw (b=b@entry=0x7f2f777fc4e0, byte_count=byte_count@entry=0x7f2f777fc558) at bitfield.c:202
        n = 0
        bits = 0x0
#4  0x00007f2f93351623 in tr_cpCreatePieceBitfield (cp=0x7f2f68003be8, byte_count=byte_count@entry=0x7f2f777fc558) at completion.c:320
        ret = <optimized out>
        n = 0
        pieces = {bits = 0x0, alloc_count = 0, bit_count = 0, true_count = 0, have_all_hint = true, have_none_hint = false}
#5  0x00007f2f933620d4 in sendBitfield (msgs=0x7f2f68186150) at peer-msgs.c:2111
        bytes = <optimized out>
        byte_count = 0
        out = 0x7f2f6813e700
#6  tellPeerWhatWeHave (msgs=0x7f2f68186150) at peer-msgs.c:2136
        fext = <optimized out>
#7  tr_peerMsgsNew (torrent=torrent@entry=0x7f2f680039b0, io=io@entry=0x7f2f68584b50, callback=callback@entry=0x7f2f93359e40 <peerCallbackFunc>, callbackData=callbackData@entry=0x7f2f68004500) at peer-msgs.c:2634
        m = 0x7f2f68186150
#8  0x00007f2f933590d2 in createBitTorrentPeer (client=406, atom=0x7f2f6800d0f0, io=0x7f2f68584b50, tor=0x7f2f680039b0) at peer-mgr.c:1932
        peer = <optimized out>
        swarm = 0x7f2f68004500
#9  myHandshakeDoneCB (handshake=<optimized out>, io=<optimized out>, readAnythingFromPeer=<optimized out>, isConnected=<optimized out>, peer_id=0x7f2f68584b7c "TIX0194-j2e4f9d8i9a9\203\t\360Q", vmanager=<optimized out>) at peer-mgr.c:2054
        client = 406
        io = 0x7f2f68584b50
        buf = "BitTornado 18.33.0\000\000\000\000\000\000`\240\003h/\177\000\000\000\001\000\000\000\000\000\000\261\071\200\210\320\200\377\377\021\000\000\000\000\000\000\000\264\177\066\223/\177\000\000P\307\177w/\177\000\000s\212\066\223/\177\000\000\000\000\000\000\000\000\000\000\024", '\000' <repeats 15 times>, "s6\207\220/\177\000\000p+Ah\022\000\000\000!\000\000\000\000\000\000"
        peer = <optimized out>
        atom = 0x7f2f6800d0f0
        ok = <optimized out>
        success = false
        port = 6732
        addr = 0x7f2f68584ba0
        manager = <optimized out>
        s = 0x7f2f68004500
#10 0x00007f2f9336a28e in fireDoneFunc (isConnected=true, handshake=0x7f2f68047c60) at handshake.c:1057
        peer_id = <optimized out>
        success = <optimized out>
#11 tr_handshakeDone (handshake=handshake@entry=0x7f2f68047c60, isOK=<optimized out>) at handshake.c:1085
No locals.
#12 0x00007f2f9336b23a in readCryptoSelect (inbuf=<optimized out>, handshake=<optimized out>) at handshake.c:536
        pad_d_len = 0
        crypto_select = 19
#13 canRead (io=<optimized out>, arg=0x7f2f68047c60, piece=<optimized out>) at handshake.c:1027
        ret = READ_NOW
        handshake = 0x7f2f68047c60
        inbuf = 0x7f2f68412b70
        readyForMore = true
#14 0x00007f2f933573a1 in canReadWrapper (io=0x7f2f68584b50) at peer-io.c:206
        oldLen = 68
        ret = <optimized out>
        overhead = <optimized out>
        piece = 0
        used = <optimized out>
        now = 1374685575793
        session = 0x7f2f93e68000

#15 0x00007f2f9337b7e5 in UTP_ProcessIncoming (conn=conn@entry=0x7f2f6828e760, packet=packet@entry=0x7f2f777fcb80 "\001", len=len@entry=88, syn=syn@entry=false) at utp.cpp:2158
        count = 68
        packet_end = 0x7f2f777fcbd8 "-u\336\246%LJY\021\200h\257\207t\372R\023?h\270\233\330\235q\216\313\251\335\374\247\223t\026\062\354\306"
        min_rtt = 9223372036854775807
        prev_delay_base = <optimized out>
        pk_ack_nr = <optimized out>
        time = <optimized out>
        their_delay = <optimized out>
        pf = 0x7f2f777fcb80
        pf1 = 0x7f2f777fcb80
        selack_ptr = 0x0
        data = <optimized out>
        extension = <optimized out>
        seqnr = 0
        acks = <optimized out>
        actual_delay = <optimized out>
        pk_seq_nr = 57245
        pk_flags = <optimized out>
        acked_bytes = 0
        p = <optimized out>

Change History (10)

comment:1 Changed 8 years ago by jordan

  • Component changed from Transmission to libtransmission
  • Owner set to jordan
  • Status changed from new to assigned

Looks like there are two bugs here. There's the low-level code that underflows in set_all_true(), but there's also the question of why we're trying to send a zero-byte-length bitfield.

Looks like the latter bug is triggered in peer-msgs.c's tellPeerWhatWeHave(). It looks to see if we have-all, have-none, or have-some. We don't even have the metadata for this torrent yet, so at this point we should be have-none. However, tr_cpHasAll() is returning true because of the vacuous case: it has all of the 0 pieces that it knows about.

tr_cpHasAll() and tr_cpHasNone() should take magnet links into account.

comment:2 Changed 8 years ago by jordan

  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed in r14151.

comment:3 Changed 8 years ago by jordan

See also a side-ticket from this ticket's backtrace: #5443

Last edited 8 years ago by jordan (previous) (diff)

comment:4 follow-up: Changed 8 years ago by cfpp2p

I get a Déjà vu pertaining to client: BitTornado?? 18.33.0-->#4935 and this ticket . It seems to me more than an insignificant chance that the same (misidentified) client: BitTornado?? 18.33.0 and uTP are both involved again. Perhaps reporter at https://bugzilla.redhat.com/show_bug.cgi?id=989258 hasn't updated per: https://trac.transmissionbt.com/ticket/5002#comment:32

Changed 8 months ago by jordan 

Ideally I'd like to pull an "official" fix from libutp's repo, and I can't tell
the context of this patch wrt it being experimental, proposed, etc... but
regardless, it looks like an improvement over trunk, so I've landed it in
r13646 for 2.74.

r13646 The developers at transmission put a heartfelt effort into solving these type of obscure bugs and for this it is much appreciated.

comment:5 in reply to: ↑ 4 Changed 8 years ago by x190

Replying to cfpp2p:

18.33.0 and uTP are both involved again. Perhaps reporter at ​https://bugzilla.redhat.com/show_bug.cgi?id=989258 hasn't updated per: #5002 Changed 8 months ago by jordan ... I've landed it in r13646 for 2.74.

Redhat user version claims to be transmission-gtk-2.80.

comment:6 Changed 8 years ago by jordan

  • Version changed from 2.81 to 2.80

Downstream's ticket was reported against 2.80, so reassigning version here

comment:7 Changed 8 years ago by x190

The following crash was submitted to me by a friend who experienced the crash using v2.82, while removing a selection of just added magnet links and regular torrents:

    Program terminated with signal 11, Segmentation fault.
    #0 tr_torGetPieceBlockRange (tor=0xe87150b7db096ec9, piece=3, first=first@entry=0x7f4008a1fc58, last=last@entry=0x7f4008a1fc5c) at torrent.c:2537
    2517 uint64_t offset = tor->info.pieceSize;



    Thread 1 (Thread 0x7f4008a20700 (LWP 26783)):
    #0 tr_torGetPieceBlockRange (tor=0xe87150b7db096ec9, piece=3, first=first@entry=0x7f4008a1fc58, last=last@entry=0x7f4008a1fc5c) at torrent.c:2537
    offset = <optimized out>
    #1 0x00007f400dbf1d03 in tr_cpMissingBlocksInPiece (cp=0x7f40047291b8, piece=<optimized out>) at completion.c:244
    f = 119
    l = 124
    #2 0x00007f400dbf0e62 in tr_cpPieceIsComplete (i=<optimized out>, cp=<optimized out>) at completion.h:117
    No locals.
    #3 getBlockRun (cache=cache@entry=0x7f400f2d4f80, pos=pos@entry=0, info=info@entry=0x7f4006448db0) at cache.c:108
    b = 0x7f4004da59a0
    i = <optimized out>
    n = <optimized out>
    blocks = <optimized out>
    ref = <optimized out>
    block = <optimized out>
    #4 0x00007f400dbf0ee6 in calcRuns (cache=cache@entry=0x7f400f2d4f80, runs=runs@entry=0x7f4006448db0) at cache.c:146
    rank = <optimized out>
    n = 853
    i = 0
    pos = 0
    now = 1375993172
    #5 0x00007f400dbf152d in tr_cacheFlushDone (cache=0x7f400f2d4f80) at cache.c:422
    i = 0
    n = <optimized out>
    runs = 0x7f4006448db0
    err = 0
    #6 0x00007f400dbd1325 in onSaveTimer (foo=<optimized out>, bar=<optimized out>, vsession=0x7f400f2d4310) at session.c:555
    tor = 0x0
    session = 0x7f400f2d4310
    #7 0x00007f400d563849 in event_process_active_single_queue (activeq=<optimized out>, base=<optimized out>) at event.c:1340
    ev = 0x7f40040053a0
    count = 1
    #8 event_process_active (base=<optimized out>) at event.c:1407
    i = 0
    c = 0
    #9 event_base_loop (base=base@entry=0x7f40040008f0, flags=flags@entry=0) at event.c:1604
    evsel = 0x7f400d797be0 <epollops>
    tv = {tv_sec = 0, tv_usec = 130}
    tv_p = <optimized out>
    res = <optimized out>
    done = 0
    retval = 0
    __func__ = "event_base_loop"
    #10 0x00007f400d564577 in event_base_dispatch (event_base=event_base@entry=0x7f40040008f0) at event.c:1435
    No locals.
    #11 0x00007f400dbe0448 in libeventThreadFunc (veh=0x7f400f2d4ff0) at trevent.c:249
    base = 0x7f40040008f0
    eh = 0x7f400f2d4ff0
    #12 0x00007f400dbd06ca in ThreadFunc (_t=0x7f400f2d5070) at platform.c:108
    t = 0x7f400f2d5070 

Refer to #5168 and #5449 and the new peer-msgs code for clues to the root cause of this issue.

Note that both sendBitfield() and tr_cpCreatePieceBitfield() assert (tr_torrentHasMetadata()), so how could the changes made to completion.h in r14151 be the solution?

  • tr_torrentHasMetadata() only checks for fileCount > 0.
  • sendBitfield()[r14114] allowed tr_cpCreatePieceBitfield() to be called with a byte_count of 0.

It seems to me we shouldn't be sending/receiving bitfield/piece data if we don't have all the metadata (tor->info).

Last edited 8 years ago by x190 (previous) (diff)

comment:8 Changed 8 years ago by x190

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:9 Changed 8 years ago by jordan

  • Resolution set to fixed
  • Status changed from reopened to closed

This reopening confuses me for several reasons:

  1. The backtrace seems to have nothing to do with this ticket. It's crashing while flushing cached piece data, and magnet links will have no cached piece data because they can't download anything until they get the metainfo and know what there is to download.
  1. The links to bugs #5168 and #5449, which seem to be unrelated to each other and also unrelated to this ticket.
  1. Bullet point #1 insinuates there's a problem with tr_torrentHasMetadata()'s implementation of testing for fileCount > 0 but I don't see the issue?
  1. Bullet point #2 is a red herring, sendBitfield() is only used by tellPeerWhatWeHave(), which calls it if-an-only-if we have piece data for the torrent, which is impossible unless we have metainfo.
  1. The summary assserts "we shouldn't be sending/receiving bitfield/piece data if we don't have 'all' the metadata" ... (1) I don't see where we're sending or receiving piece data without metadata, (2) I don't see where we send bitfield data without metadata, (3) We can't control whether or not peers send us bitfield data before we have metainfo, (4) I don't see what any of these has to do with the backtrace you provided?

Re-closing this ticket as fixed because this all appears to be invalid. If there's something that ties all these pieces together please reopen with more information.

comment:10 Changed 8 years ago by x190

Ref: r14169

Note: See TracTickets for help on using tickets.