Opened 3 years ago

Closed 3 years ago

#23355 closed enhancement (implemented)

prop224: Implement a client purge state for NEWNYM

Reported by: dgoulet Owned by: dgoulet
Priority: High Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: prop224, tor-hs, review-group-23
Cc: Actual Points:
Parent ID: #23300 Points:
Reviewer: asn Sponsor:

Description

Similar to rend_client_purge_state()

Child Tickets

Change History (7)

comment:1 Changed 3 years ago by dgoulet

Reviewer: asn
Status: newneeds_review

Branch ticket23355_032_01

comment:2 Changed 3 years ago by asn

Patch looks good. One thing makes me anxious: We call DIGEST256MAP_FOREACH_MODIFY on so many digestmaps without first checking that they are not-NULL. I don't think the foreach functions check for NULL before iterating which might cause segfaults. Shouldn't we check that the maps are non-NULL?

Other than that, LGTM.

Feel free to fix the above or not and then mark it as merge_ready.

comment:3 Changed 3 years ago by dgoulet

Status: needs_reviewmerge_ready

These maps are caches initialized by hs_cache_init() which is called by hs_init() in tor_init().

If they are NULL, tor is in a really really bad shape. I think there are some levels of confidence we can accept here that is assuming they won't be NULL. Else, our entire testing process is badly flawed if we can't notice an unallocated cache and nothing segfaulted? :)

Because to that extent, we have a LOT of callsite that assume that what is initialized in init() functions are safe to use at runtime (just see rendcache.c and friends).

comment:4 Changed 3 years ago by nickm

Keywords: review-group-23 added

Put 0.3.2 needs_review and merge_ready tickets into review-group-23.

comment:5 Changed 3 years ago by dgoulet

Owner: set to dgoulet
Status: merge_readyaccepted

comment:6 Changed 3 years ago by dgoulet

Status: acceptedmerge_ready

comment:7 Changed 3 years ago by nickm

Resolution: implemented
Status: merge_readyclosed

confirmed with asn, and merging

Note: See TracTickets for help on using tickets.