Opened 4 months ago

Closed 4 months ago

#30799 closed defect (fixed)

Fix a small memory leak in nt_service_install() in ntmain.c

Reported by: xiaoyinl Owned by:
Priority: Very Low Milestone: Tor: 0.4.2.x-final
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Trivial Keywords: no-backport, 041-can, memory-management, unreachable-code
Cc: Actual Points: 0.1
Parent ID: Points: 0.1
Reviewer: teor Sponsor:

Description

A string buffer command is allocated on L559, but it's not freed if service_fns.LookupAccountNameA_fn returns FALSE. The patch I attached adds tor_free(command); in this else if branch.

Child Tickets

Attachments (1)

0001-Free-a-string-buffer-in-nt_service_install.patch (931 bytes) - added by xiaoyinl 4 months ago.
patch

Download all attachments as: .zip

Change History (8)

Changed 4 months ago by xiaoyinl

patch

comment:1 Changed 4 months ago by nickm

Keywords: ???-backport 041-can added
Milestone: Tor: 0.4.1.x-final
Status: newneeds_review

comment:2 Changed 4 months ago by dgoulet

Reviewer: teor

comment:3 Changed 4 months ago by teor

Actual Points: 0.1
Keywords: no-backport memory-management unreachable-code added; ???-backport removed
Milestone: Tor: 0.4.1.x-finalTor: 0.4.2.x-final
Points: 0.1
Priority: LowVery Low
Severity: MinorTrivial
Status: needs_reviewmerge_ready

Thanks for this patch!

I made it into a pull request at:
https://github.com/torproject/tor/pull/1100

The fix looks good to me.

This code is not reachable in any supported tor version, so we don't need to backport the fix.

comment:4 Changed 4 months ago by teor

Please check the pull request before merging:
https://github.com/torproject/tor/pull/1100

comment:5 Changed 4 months ago by teor

Status: merge_readyneeds_revision

This patch needs a practracker fix. And my git directory needs the latest pre-push hook.

comment:6 Changed 4 months ago by teor

Status: needs_revisionmerge_ready

I did the changes file and the practracker fix, so this is now ready for merge.

comment:7 Changed 4 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

Merged to master.

Note: See TracTickets for help on using tickets.