Bug 2401 - sshd copying options leaks memory
Summary: sshd copying options leaks memory
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: sshd (show other bugs)
Version: 6.8p1
Hardware: Other Linux
: P5 enhancement
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-20 16:38 AEST by Jakub Jelen
Modified: 2021-04-23 14:59 AEST (History)
0 users

See Also:


Attachments
proposed patch (813 bytes, text/plain)
2015-05-20 16:38 AEST, Jakub Jelen
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelen 2015-05-20 16:38:27 AEST
Created attachment 2623 [details]
proposed patch

To continue with my thematic reports about memory usage, valgrind showed up some more memory problems around handling server options. And after some basic understanding of what is going on around there, I came to this upstream commit:

https://anongit.mindrot.org/openssh.git/commit/?id=d8478b6a9b32760d47c2419279c4a73f5f88fdb6

The core of problem is that when you read options from file, you allocate memory for them and then you copy them to another options structure using xstrdup, which leaves behind two copies abandoned. One is still left in old array (never freed) and the other is overwritten by a copy you are assigning to the target array.

Proposing patch that handles memory proper way:
 1) In first cycle go through the target array and free all strings
 2) In second cycle move values from first array to the other
It requires one more variable to operate (chosen classic i). Fortunately, the macro can't be used anywhere else so we can depend on local variable.
Comment 1 Jakub Jelen 2018-03-27 01:17:14 AEDT
This looks like resolved by the following upstream commit:

https://github.com/openssh/openssh-portable/commit/dceabc7ad

Thank you for taking care of this.
Comment 2 Damien Miller 2021-04-23 14:59:54 AEST
closing resolved bugs as of 8.6p1 release