Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
-+-
 Reporter:  cyberpunks   |  Owner:  ahf
 Type:  defect   | Status:  closed
 Priority:  Very High|  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.3.3.1-alpha
 Severity:  Normal   | Resolution:  fixed
 Keywords:  rust, protover, memory-safety,   |  Actual Points:
  035-must, fast-fix, 033-backport,  |
  034-backport   |
Parent ID:   | Points:
 Reviewer:  catalyst |Sponsor:
-+-
Changes (by nickm):

 * status:  merge_ready => closed
 * resolution:   => fixed


Comment:

 Thanks; merged it!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
-+-
 Reporter:  cyberpunks   |  Owner:  ahf
 Type:  defect   | Status:
 |  merge_ready
 Priority:  Very High|  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.3.3.1-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, protover, memory-safety,   |  Actual Points:
  035-must, fast-fix, 033-backport,  |
  034-backport   |
Parent ID:   | Points:
 Reviewer:  catalyst |Sponsor:
-+-
Changes (by catalyst):

 * status:  needs_review => merge_ready


Comment:

 Looks good to me!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
-+-
 Reporter:  cyberpunks   |  Owner:  ahf
 Type:  defect   | Status:
 |  needs_review
 Priority:  Very High|  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.3.3.1-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, protover, memory-safety,   |  Actual Points:
  035-must, fast-fix, 033-backport,  |
  034-backport   |
Parent ID:   | Points:
 Reviewer:  catalyst |Sponsor:
-+-
Changes (by asn):

 * reviewer:   => catalyst


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
-+-
 Reporter:  cyberpunks   |  Owner:  ahf
 Type:  defect   | Status:
 |  needs_review
 Priority:  Very High|  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.3.3.1-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, protover, memory-safety,   |  Actual Points:
  035-must, fast-fix, 033-backport,  |
  034-backport   |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-
Changes (by nickm):

 * reviewer:  nickm =>


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
-+-
 Reporter:  cyberpunks   |  Owner:  ahf
 Type:  defect   | Status:
 |  needs_review
 Priority:  Very High|  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.3.3.1-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, protover, memory-safety,   |  Actual Points:
  035-must, fast-fix, 033-backport,  |
  034-backport   |
Parent ID:   | Points:
 Reviewer:  nickm|Sponsor:
-+-

Comment (by nickm):

 I've tried to restore the fix in `bug27740_035_fix`; PR at
 https://github.com/torproject/tor/pull/458 . How does that look?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
-+-
 Reporter:  cyberpunks   |  Owner:  ahf
 Type:  defect   | Status:
 |  needs_review
 Priority:  Very High|  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.3.3.1-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, protover, memory-safety,   |  Actual Points:
  035-must, fast-fix, 033-backport,  |
  034-backport   |
Parent ID:   | Points:
 Reviewer:  nickm|Sponsor:
-+-

Comment (by cyberpunks):

 Replying to [comment:14 nickm]:
 > (Does that look okay?)

 No,
 
[https://gitweb.torproject.org/tor.git/commit/?id=289a7dbac32a981897e12a3c250f0b6c67eec809
 that merge] completely undid all the changes in
 
[https://gitweb.torproject.org/tor.git/commit/?id=42558df7c8affeec33e66d987ccf4d632a9d5466
 42558df7c8affeec33e66d987ccf4d632a9d5466] that fixed this bug.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
-+-
 Reporter:  cyberpunks   |  Owner:  ahf
 Type:  defect   | Status:
 |  needs_review
 Priority:  Very High|  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.3.3.1-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, protover, memory-safety,   |  Actual Points:
  035-must, fast-fix, 033-backport,  |
  034-backport   |
Parent ID:   | Points:
 Reviewer:  nickm|Sponsor:
-+-

Comment (by nickm):

 Merged, with a conflict resolved in
 289a7dbac32a981897e12a3c250f0b6c67eec809.  (Does that look okay?)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
-+-
 Reporter:  cyberpunks   |  Owner:  ahf
 Type:  defect   | Status:
 |  needs_review
 Priority:  Very High|  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.3.3.1-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, protover, memory-safety,   |  Actual Points:
  035-must, fast-fix, 033-backport,  |
  034-backport   |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-
Changes (by teor):

 * status:  assigned => needs_review


Comment:

 I think this has a patch, so it needs review?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
-+-
 Reporter:  cyberpunks   |  Owner:  (none)
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.3.3.1-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, protover, memory-safety,   |  Actual Points:
  035-must, fast-fix, 033-backport,  |
  034-backport   |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by cyberpunks):

 https://gitgud.io/onionk/tor/compare/master...rust-allsupportednull1

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
-+-
 Reporter:  cyberpunks   |  Owner:  (none)
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.3.3.1-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, protover, memory-safety,   |  Actual Points:
  035-must, fast-fix, 033-backport,  |
  034-backport   |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by teor):

 Replying to [comment:6 cyberpunks]:
 > Rust uses jemalloc to allocate memory by default. No CI has been failing
 even though `test_protover_all_supported` calls `tor_free()` on this
 jemalloc-allocated pointer though. Is it missing hardening that could
 catch similar issues in the future?

 ASAN would probably help, but it doesn't work with jemalloc, see #27272.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
-+-
 Reporter:  cyberpunks   |  Owner:  (none)
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.3.3.1-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, protover, memory-safety,   |  Actual Points:
  035-must, fast-fix, 033-backport,  |
  034-backport   |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by cyberpunks):

 Rust uses jemalloc to allocate memory by default. No CI has been failing
 even though `test_protover_all_supported` calls `tor_free()` on this
 jemalloc-allocated pointer though. Is it missing hardening that could
 catch similar issues in the future?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
-+-
 Reporter:  cyberpunks   |  Owner:  (none)
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.3.3.1-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, protover, memory-safety,   |  Actual Points:
  035-must, fast-fix, 033-backport,  |
  034-backport   |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by teor):

 Replying to [comment:5 cyberpunks]:
 > Replying to [comment:3 teor]:
 > > As far as I understand it, it may be ok to allocate in Rust and
 deallocate in C, as long as they use the same allocator.
 > Oh right. Of course currently, they don't.

 We have allocate_and_copy_string() in Rust, which uses tor_malloc():
 
https://github.com/torproject/tor/blob/master/src/rust/tor_allocate/tor_allocate.rs#L30

 We need to update our RustCodingStandards.md to mention
 allocate_and_copy_string():
 
https://gitweb.torproject.org/tor.git/tree/doc/HACKING/CodingStandardsRust.md#n374

 I'll open another ticket.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
-+-
 Reporter:  cyberpunks   |  Owner:  (none)
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.3.3.1-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, protover, memory-safety,   |  Actual Points:
  035-must, fast-fix, 033-backport,  |
  034-backport   |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-

Comment (by cyberpunks):

 Replying to [comment:3 teor]:
 > As far as I understand it, it may be ok to allocate in Rust and
 deallocate in C, as long as they use the same allocator.
 Right, and currently, they don't.


 > deallocated in Rust (when the function returns),
 It actually isn't.

 > * when missing_out is NULL, Rust still assigns to it
 Nice catch!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
-+-
 Reporter:  cyberpunks   |  Owner:  (none)
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.3.3.1-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, protover, memory-safety,   |  Actual Points:
  035-must, fast-fix, 033-backport,  |
  034-backport   |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-
Changes (by teor):

 * keywords:  rust, protover, memory-safety, 035-must, fast-fix =>
 rust, protover, memory-safety, 035-must, fast-fix, 033-backport,
 034-backport


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
-+-
 Reporter:  cyberpunks   |  Owner:  (none)
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  0.3.5.x-final
Component:  Core Tor/Tor |Version:  Tor:
 |  0.3.3.1-alpha
 Severity:  Normal   | Resolution:
 Keywords:  rust, protover, memory-safety,   |  Actual Points:
  035-must, fast-fix |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-
Changes (by teor):

 * keywords:   => rust, protover, memory-safety, 035-must, fast-fix
 * milestone:  Tor: unspecified => Tor: 0.3.5.x-final


Comment:

 Thanks for this bug report.

 As far as I understand it, it may be ok to allocate in Rust and deallocate
 in C, as long as they use the same allocator. But, this behaviour is not
 guaranteed to be safe in future Rust releases:
 
https://gitweb.torproject.org/tor.git/tree/doc/HACKING/CodingStandardsRust.md#n365

 But even if allocating in Rust and freeing in C was safe, this function is
 also memory unsafe because:
 * *missing_out is allocated in Rust, deallocated in Rust (when the
 function returns), used in C, and then freed in C
 * when missing_out is NULL, Rust still assigns to it

 I'll open child tickets for these issues.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:  Tor: unspecified
Component:  Core Tor/Tor  |Version:  Tor: 0.3.3.1-alpha
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by cyberpunks):

 This needs the keywords `rust, protover, memory-safety`. You can't
 `tor_free()` a string allocated by rust code, which is what the C code
 will immediately try to do.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #27740 [Core Tor/Tor]: rust protover_all_supported() returns rust-allocated string in *missing_out

[Tor Bug Tracker & Wiki]

#27740: rust protover_all_supported() returns rust-allocated string in 
*missing_out
--+
 Reporter:  cyberpunks|  Owner:  (none)
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:  Tor: unspecified
Component:  Core Tor/Tor  |Version:  Tor: 0.3.3.1-alpha
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by dgoulet):

 * milestone:   => Tor: unspecified


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs