Opened 14 years ago

Closed 14 years ago

#879 closed Bug (wontfix)

[PATCH] fix segfault if tracker is freed immediately after starting a torrent

Reported by: 09BKyfYz0mi3WBjXLUV1VWo Owned by: charles
Priority: Normal Milestone: None Set
Component: libtransmission Version: 1.11
Severity: Normal Keywords:
Cc:

Description

I use libtransmission in another project of mine and I'd like to use torrentfiles not containing trackers (well, containing fake-trackers, as parsing a torrentfile without proper announce headers is not supported by libtransmission as far as I could see that when reading the source) and supply my own peers.

Doing this, I noticed that libtransmission segfaults (at least) when starting/stopping torrents because it doesn't check the ->tracker-pointer to be not NULL. I've attached a patch which adds these checks.

Attachments (1)

fixsegf.patch (2.8 KB) - added by 09BKyfYz0mi3WBjXLUV1VWo 14 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 14 years ago by charles

  • Component changed from Transmission to libtransmission
  • Milestone changed from None Set to 1.20
  • Owner set to charles
  • Status changed from new to assigned
  • Version set to 1.11

I moved the null check inside tracker.c in r5626.

comment:2 Changed 14 years ago by charles

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

comment:3 Changed 14 years ago by 09BKyfYz0mi3WBjXLUV1VWo

  • Resolution fixed deleted
  • Status changed from closed to reopened

Thanks for fixing this so fast. I noticed however that this affects also the statistics in tr_torrentStat(). See the attached new patch for a proposed fix (YMMV).

Changed 14 years ago by 09BKyfYz0mi3WBjXLUV1VWo

comment:4 Changed 14 years ago by charles

Just to be clear about this... tr_torrent having a NULL tracker pointer isn't something that ever happens in libtransmission, right? This is only happening because of changes you're making to the code to accommodate what you're working on?

comment:5 Changed 14 years ago by charles

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

comment:6 Changed 14 years ago by charles

  • Milestone changed from 1.20 to None Set

comment:7 Changed 14 years ago by 09BKyfYz0mi3WBjXLUV1VWo

  • Resolution invalid deleted
  • Status changed from closed to reopened

Right. I think that it should be possible to use libtransmission without trackers. Also, I'm only using libtransmission-functions to delete the tracker, nothing hacked into the library or something like that.

comment:8 Changed 14 years ago by charles

how? there's not an API to delete the tracker. The only place the tracker pointer is changed is in torrentRealInit() and freeTorrent(), and tr_torrent's fields are opaque.

comment:9 Changed 14 years ago by 09BKyfYz0mi3WBjXLUV1VWo

By using:

tr_trackerFree(tor->tracker);
tor->tracker = NULL;

Is this internal API?

comment:10 Changed 14 years ago by livings124

The bittorrent specifications say a torrent file has to have a tracker address. We should avoid expanding libtransmission outside the scope of what it's intended to do.

comment:11 Changed 14 years ago by 09BKyfYz0mi3WBjXLUV1VWo

As far as I know things like Peer Exchange are not in the specification either or at least they are heavily dependand on the implementation of the client. Also, I'm not asking you to change libtransmission so that it accepts trackerless files. I'm only wanting to ensure that trackers can be deleted if the client program does not need them for whatever reason (private peers, DHT, ...)

comment:12 Changed 14 years ago by charles

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

I'm happy that you're finding libtransmission useful, but tr_torrent.tracker should be considered a private field, so IMO this ticket isn't a good idea.

comment:13 Changed 14 years ago by 09BKyfYz0mi3WBjXLUV1VWo

  • Resolution invalid deleted
  • Status changed from closed to reopened

Sorry to re-open this all the time, maybe you have an IRC-channel or email address for better discussion?

Are you happy with the current "trackerless", DHT-only solution? Why do you want to keep trackers in the library, as there is no obvious reason to me (besides implementing like others do, what can hardly be an argument) if libtransmission is interpreted as a torrent library and not only a .torrent-file-processor. It works fine with "self-managed" peers (given that the little patch I've attached above is applied).

I don't want to bug you, but we've chosen libtransmission for our project because it's the best torrent library. If we cannot find a solution for this in upstream, we would need to deliver our own version of libtransmission which would be a bunch of work for us and an inconvenience for the user, so we want to avoid that by all means.

comment:14 Changed 14 years ago by charles

I'm not sure what you mean by your last message, we don't currently have DHT-only solutions because we don't support DHT yet.

If I'm reading this right and you're talking about adding DHT support to libtransmission, I'd be very interested in that and be willing to give a lot more leeway with the codebase.

comment:15 Changed 14 years ago by livings124

The irc channel is #transmission on freenode

comment:16 Changed 14 years ago by 09BKyfYz0mi3WBjXLUV1VWo

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

As discussed in IRC i'm going to submit exactly one patch with a fully working peer-management "from outside" to make this not an never-ending story.

Ticket will be re-opened as soon as the patch is ready.

Note: See TracTickets for help on using tickets.