Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#5912 closed Bug (fixed)

corrupt blocks overwrite previously verified OK blocks

Reported by: cfpp2p Owned by: mike.dld
Priority: Normal Milestone: 2.90
Component: libtransmission Version: 2.84
Severity: Normal Keywords:
Cc:

Description

I can verify that there is indeed a bug in webseed.c where corrupt blocks overwrite previously deemed verified OK block of a piece. I have test torrents that will consistently reproduce verified corrupt blocks after 100% completion, and then a state of less than 100%. The fix is having tr_torrentPieceIsComplete() test before the two instances of tr_cacheWriteBlock() in webseed.c

This fault was introduced with r12539. See #4649 for related information.

something like this will do it

static void
write_block_func (void * vdata)
{
  struct write_block_data * data = vdata;
  struct tr_webseed * w = data->webseed;
  struct evbuffer * buf = data->content;
  struct tr_torrent * tor;

  tor = tr_torrentFindFromId (data->session, data->torrent_id);
  if (tor != NULL)
    {
      const uint32_t block_size = tor->blockSize;
      uint32_t len = evbuffer_get_length (buf);
      const uint32_t offset_end = data->block_offset + len;
      tr_cache * cache = data->session->cache;
      const tr_piece_index_t piece = data->piece_index;
     
      if (!tr_torrentPieceIsComplete (tor, piece))
   {
     while (len > 0)
       {
         const uint32_t bytes_this_pass = MIN (len, block_size);
         tr_cacheWriteBlock (cache, tor, piece, offset_end - len, bytes_this_pass, buf);
         len -= bytes_this_pass;
       }
      
     tr_bitfieldAdd (&w->blame, piece);
     fire_client_got_blocks (tor, w, data->block_index, data->count);
   }
    }

  evbuffer_free (buf);
  tr_free (data);
}
static void
web_response_func (tr_session    * session,
         char       * task_addr,
         bool         aborted,
                   bool            did_connect UNUSED,
                   bool            did_timeout,
                   long            response_code,
                   const void    * response UNUSED,
                   size_t          response_byte_count UNUSED,
                   void          * vtask)
{
...

           else
            {
              if (buf_len && !tr_torrentPieceIsComplete (tor, t->piece_index))
                {
                  /* on_content_changed () will not write a block if it is smaller than
                     the torrent's block size, i.e. the torrent's very last block */
                  tr_cacheWriteBlock (session->cache, tor,
                                      t->piece_index, t->piece_offset + bytes_done,
                                      buf_len, t->content);
        tr_bitfieldAdd (&w->blame, t->piece_index);
                  fire_client_got_blocks (tor, t->webseed,
                                          t->block + t->blocks_done, 1);
                }

Change History (7)

comment:1 Changed 6 years ago by x190

One should apologize for cf's lack of attention to proper formatting. Also, the tr_bitfieldAdd() lines in each code block and the "char * task_addr, bool aborted," lines in the second block are from my upcoming webseed patch, which also means "bool did_timeout" should be "bool did_timeout UNUSED" for this ticket.

See #5923 for a patch for this and many more webseed issues.

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

comment:2 Changed 6 years ago by cfpp2p

x190's thorough testing found the bug, and regardless the bug exists in current trunk and prior versions back to at least r12539. The original fix that I came up with, in its simplest form, fixes trunk as well as x190's 'upcoming webseed patch'. Click the Unified tab below to see in well formatted view.

  • J:/T~~/5912/webseed.c

    old new  
    162162      tr_cache * cache = data->session->cache;
    163163      const tr_piece_index_t piece = data->piece_index;
    164164
    165       while (len > 0)
     165      if (!tr_torrentPieceIsComplete (tor, piece))
    166166        {
    167           const uint32_t bytes_this_pass = MIN (len, block_size);
    168           tr_cacheWriteBlock (cache, tor, piece, offset_end - len, bytes_this_pass, buf);
    169           len -= bytes_this_pass;
     167          while (len > 0)
     168            {
     169              const uint32_t bytes_this_pass = MIN (len, block_size);
     170              tr_cacheWriteBlock (cache, tor, piece, offset_end - len, bytes_this_pass, buf);
     171              len -= bytes_this_pass;
     172            }
     173          fire_client_got_blocks (tor, w, data->block_index, data->count);
    170174        }
    171 
    172       fire_client_got_blocks (tor, w, data->block_index, data->count);
    173175    }
    174176
    175177  evbuffer_free (buf);
     
    422424            }
    423425            else
    424426            {
    425               if (buf_len)
     427              if (buf_len && !tr_torrentPieceIsComplete (tor, t->piece_index))
    426428                {
    427429                  /* on_content_changed () will not write a block if it is smaller than
    428430                     the torrent's block size, i.e. the torrent's very last block */

The bug can easily be reproduced and likely produced some of the complaints dictated in #4649

comment:3 follow-up: Changed 6 years ago by rsully

Does this only affect users using web seeds, what is required to reproduce it?

comment:4 in reply to: ↑ 3 Changed 6 years ago by cfpp2p

Replying to rsully:

Does this only affect users using web seeds, what is required to reproduce it?

Yes, web seeds.

The requirements to reproduce are at least one webseed that sends corrupt block(s), and at least one other peer (webseed or regular peer) that sends correct data for the same block.

Then a peer with correct block sends the block and transmission accepts and verifies that block as good. But then the webseed thread comes around and the same block is downloaded from the webseed that sends a bad block which is then written even though the piece was previously deemed complete.

It may not happen every time, but if the required elements of the situation are there it will happen frequently and easily enough for proof.

I did quite a lot of testing and research with this issue and other webseed issues. It's been a few weeks now so I can't really remember any better than that right now.

comment:6 Changed 5 years ago by mike.dld

  • Owner changed from jordan to mike.dld
  • Status changed from new to assigned

comment:7 Changed 5 years ago by mike.dld

  • Milestone changed from None Set to 2.90
  • Resolution set to fixed
  • Status changed from assigned to closed

Committed as r14550, thanks.

Note: See TracTickets for help on using tickets.