[tor-bugs] #25306 [Core Tor/Tor]: tor_assertion_failed_(): Bug: ../src/or/hs_service.c:1985: rotate_all_descriptors: Assertion service->desc_current failed; aborting.

[Tor Bug Tracker & Wiki]

#25306: tor_assertion_failed_(): Bug: ../src/or/hs_service.c:1985:
rotate_all_descriptors: Assertion service->desc_current failed; aborting.
------------------------------------------------+--------------------------
 Reporter:  cypherpunks                         |          Owner:  (none)
     Type:  defect                              |         Status:  new
 Priority:  Medium                              |      Milestone:  Tor:
                                                |  0.3.3.x-final
Component:  Core Tor/Tor                        |        Version:  Tor:
                                                |  0.3.3.2-alpha
 Severity:  Normal                              |     Resolution:
 Keywords:  tor-hs crash 033-must 032-backport  |  Actual Points:
Parent ID:                                      |         Points:
 Reviewer:                                      |        Sponsor:
------------------------------------------------+--------------------------

Comment (by dgoulet):

 Hmmmmmm there is something very weird in `rotate_all_descriptors()`.

 See the comment in this code snippet:

 {{{
     /* Note for a service booting up: Both descriptors are NULL in that
 case
      * so this function might return true if we are in the timeframe for a
      * rotation leading to basically swapping two NULL pointers which is
      * harmless. However, the side effect is that triggering a rotation
 will
      * update the service state and avoid doing anymore rotations after
 the
      * two descriptors have been built. */
     if (!should_rotate_descriptors(service, now)) {
       continue;
     }

     tor_assert(service->desc_current);
     tor_assert(service->desc_next);
 }}}

 We do rotation _before_ building descriptors (assuming we are booting up).
 The comment mentions that it is possible to have NULL descriptors at first
 and that is OK because we just swap NULL pointers.

 But then why those asserts!? Comes from commit `cd07af60c9e` but it
 doesn't tell me much why they were put there especially with such a
 comment.

 I think those asserts() should *not* be there and we haven't hit those yet
 because when tor boots up, I bet we are able to build the descriptors
 before we get the latest consensus so when we end up in a situation where
 `should_rotate_descriptors()` returns true, we have the descriptors.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/25306#comment:6>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online