Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: OpenSSH: Dev

Potential memory leak in sshd [detected by melton]

  

 
OpenSSH dev RSS feed   Index | Next | Previous | View Threaded


zhenbo1987 at gmail

Feb 3, 2012, 5:55 PM

Post #1 of 19 (1425 views)
Permalink
Potential memory leak in sshd [detected by melton]
Hi all,

After the memory leaks (bug 1967
<https://bugzilla.mindrot.org/show_bug.cgi?id=1967>) I reported in
bugzilla are fixed,

I also applied melton(http://lcs.ios.ac.cn/~xuzb/melton.html)

to detect the potential bugs in sshd (openssh-5.9p1).


The url below is the index of bug reports that are checked as real
bugs manually.

http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/index.html


Shall we fix these bugs? Or just let them go since they are not so serious?

Hope for your replies!


--
Zhenbo Xu
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


zhenbo1987 at gmail

Feb 3, 2012, 6:02 PM

Post #2 of 19 (1386 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
By the way, I submitted this report in bugzilla a few days ago, but there
is no response.
Should I report bugs in this mailing list rather than in bugzilla?

2012/2/4 Zhenbo Xu <zhenbo1987 [at] gmail>

> Hi all,
>
> After the memory leaks (bug 1967 <https://bugzilla.mindrot.org/show_bug.cgi?id=1967>) I reported in bugzilla are fixed,
>
> I also applied melton(http://lcs.ios.ac.cn/~xuzb/melton.html)
>
> to detect the potential bugs in sshd (openssh-5.9p1).
>
>
> The url below is the index of bug reports that are checked as real bugs manually.
>
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/index.html
>
>
> Shall we fix these bugs? Or just let them go since they are not so serious?
>
> Hope for your replies!
>
>
> --
> Zhenbo Xu
>



--
Zhenbo Xu
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


dan at doxpara

Feb 3, 2012, 6:21 PM

Post #3 of 19 (1389 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
ssh uses a unique process for each connection, and sshd forks for each new
connection, so the only memory leaks that are "in scope" are those that
apply to the master sshd. I don't think that applies to any of these bugs.

Melton looks rather cool though.

On Fri, Feb 3, 2012 at 9:02 PM, Zhenbo Xu <zhenbo1987 [at] gmail> wrote:

> By the way, I submitted this report in bugzilla a few days ago, but there
> is no response.
> Should I report bugs in this mailing list rather than in bugzilla?
>
> 2012/2/4 Zhenbo Xu <zhenbo1987 [at] gmail>
>
> > Hi all,
> >
> > After the memory leaks (bug 1967 <
> https://bugzilla.mindrot.org/show_bug.cgi?id=1967>) I reported in
> bugzilla are fixed,
> >
> > I also applied melton(http://lcs.ios.ac.cn/~xuzb/melton.html)
> >
> > to detect the potential bugs in sshd (openssh-5.9p1).
> >
> >
> > The url below is the index of bug reports that are checked as real bugs
> manually.
> >
> >
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/index.html
> >
> >
> > Shall we fix these bugs? Or just let them go since they are not so
> serious?
> >
> > Hope for your replies!
> >
> >
> > --
> > Zhenbo Xu
> >
>
>
>
> --
> Zhenbo Xu
> _______________________________________________
> openssh-unix-dev mailing list
> openssh-unix-dev [at] mindrot
> https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
>
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


zhenbo1987 at gmail

Feb 4, 2012, 9:54 PM

Post #4 of 19 (1382 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
What about this report
http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-uWzwb1.html#EndPath
?
At the end of the function server_accept_loop, fdset leaks. It means each
time the master sshd accept a new connection, it create a new space for
fdset but no free it. Or it is a false positive?

2012/2/4 Dan Kaminsky <dan [at] doxpara>

> ssh uses a unique process for each connection, and sshd forks for each new
> connection, so the only memory leaks that are "in scope" are those that
> apply to the master sshd. I don't think that applies to any of these bugs.
>
> Melton looks rather cool though.
>
> On Fri, Feb 3, 2012 at 9:02 PM, Zhenbo Xu <zhenbo1987 [at] gmail> wrote:
>
>> By the way, I submitted this report in bugzilla a few days ago, but there
>> is no response.
>> Should I report bugs in this mailing list rather than in bugzilla?
>>
>> 2012/2/4 Zhenbo Xu <zhenbo1987 [at] gmail>
>>
>> > Hi all,
>> >
>> > After the memory leaks (bug 1967 <
>> https://bugzilla.mindrot.org/show_bug.cgi?id=1967>) I reported in
>> bugzilla are fixed,
>>
>> >
>> > I also applied melton(http://lcs.ios.ac.cn/~xuzb/melton.html)
>> >
>> > to detect the potential bugs in sshd (openssh-5.9p1).
>> >
>> >
>> > The url below is the index of bug reports that are checked as real bugs
>> manually.
>> >
>> >
>> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/index.html
>> >
>> >
>> > Shall we fix these bugs? Or just let them go since they are not so
>> serious?
>> >
>> > Hope for your replies!
>> >
>> >
>> > --
>> > Zhenbo Xu
>> >
>>
>>
>>
>> --
>> Zhenbo Xu
>> _______________________________________________
>> openssh-unix-dev mailing list
>> openssh-unix-dev [at] mindrot
>> https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
>>
>
>


--
Zhenbo Xu
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


keisial at gmail

Feb 5, 2012, 12:38 PM

Post #5 of 19 (1379 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
On 05/02/12 06:54, Zhenbo Xu wrote:
> What about this report
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-uWzwb1.html#EndPath
> ?
> At the end of the function server_accept_loop, fdset leaks. It means each
> time the master sshd accept a new connection, it create a new space for
> fdset but no free it. Or it is a false positive?
No. Before allocating the fdset (line 14156), it frees the previous one
if present (line 14155).

There's a small leak of the last one when it exits the infinite loop. It
should probably free fdset on line 14330.

It seems an odd way, doing the free() + calloc() on every iteration.
Seems easier to do
if (fdset != NULL)
bzero(fdset, ...);
else
fdset = xcalloc(...);

maxfd can be decreased in line 14236, but that doesn't require the
xfree() + xcalloc() pattern.


_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


keisial at gmail

Feb 5, 2012, 12:51 PM

Post #6 of 19 (1381 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
On 04/02/12 02:55, Zhenbo Xu wrote:
> I also applied melton(http://lcs.ios.ac.cn/~xuzb/melton.html)
>
> to detect the potential bugs in sshd (openssh-5.9p1).
>
>
> The url below is the index of bug reports that are checked as real
> bugs manually.
>
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/index.html
>
>
> Shall we fix these bugs? Or just let them go since they are not so serious?
> Hope for your replies!

The second leak

Logic error Memory leak session.i 13193 13 View Report
<http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-zSMfqI.html#EndPath>


http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-zSMfqI.html#EndPath

is a false positive.

The only way to exit the infinite loop is the return of line 13267.
And line 13266 calls session_close(), which frees s->auth_data in line
14994.
(granted, it's tricky to follow...)

The analysis seem to have stopped in line 13193, after step 14.

As a suggestion, I recommend you to make the messages "Execution
continues on line 12345"
links to line 12345.
Also, going to the opening brace from the closing one would be useful
when dealing with big blocks.

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


keisial at gmail

Feb 5, 2012, 1:02 PM

Post #7 of 19 (1383 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
On 04/02/12 02:55, Zhenbo Xu wrote:
> The url below is the index of bug reports that are checked as real
> bugs manually.
>
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/index.html
>
>
> Shall we fix these bugs? Or just let them go since they are not so serious?
>
> Hope for your replies!
The third error
Logic error Memory leak monitor.i 13658 3
seems like a good catch.
There should be a call to buffer_free(&logmsg); before the return -1 of
monitor_read_log.

Although it only happens if the client closed the socket, in which case
the next poll of
line 13702 should fail and monitor_read_log never called again.
So it probably only leaks once.

Attaching fix.
Attachments: monitor_read_log-buffer_free.patch (0.37 KB)


keisial at gmail

Feb 5, 2012, 1:11 PM

Post #8 of 19 (1379 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
On 05/02/12 21:51, 聲gel Gonz嫮ez wrote:
> The second leak
>
> Logic error Memory leak session.i 13193 13 View Report
> <http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-zSMfqI.html#EndPath>
>
>
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-zSMfqI.html#EndPath
>
> is a false positive.
>
> The only way to exit the infinite loop is the return of line 13267.
> And line 13266 calls session_close(), which frees s->auth_data in line
> 14994.


The fifth one is a duplicate of this (same path, but the variable
allocated on next line).
12th one is the same issue of freeing at session_close not detected. But
this time with s->auth_display
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


keisial at gmail

Feb 5, 2012, 1:34 PM

Post #9 of 19 (1380 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
On 04/02/12 02:55, Zhenbo Xu wrote:
> The url below is the index of bug reports that are checked as real
> bugs manually.
>
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/index.html
>
> Shall we fix these bugs? Or just let them go since they are not so serious?
>
> Hope for your replies!

The seventh report is not complete, but seem a genuine leak:
Logic error Memory leak auth2-chall.i 9868 5 View Report
<http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-0o5DSr.html#EndPath>


http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-0o5DSr.html#EndPath

Melton complains that authctxt->kbdintctxt is never freed.
The return value passes authctxt to auth2_challenge_start(), and
auth2_challenge_start() may call
auth2_challenge_stop(), which frees it (line 9878) [melton doesn't
realise this].
In the path where it doesn't free it, it sets authctxt->postponed and
returns 0 to userauth_kbdint(),
itself called from input_userauth_request() at auth2.c line 283.
There then calls userauth_finish() at line 285. Which return in line 336.
And input_userauth_request() finishes with authctxt getting out of
scope, without authctxt->kbdintctxt
having been freed.

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


keisial at gmail

Feb 5, 2012, 1:58 PM

Post #10 of 19 (1380 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
On 04/02/12 02:55, Zhenbo Xu wrote:
> The url below is the index of bug reports that are checked as real
> bugs manually.
>
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/index.html
>
>
> Shall we fix these bugs? Or just let them go since they are not so serious?
>
> Hope for your replies!
The 8th report is another false positive
Logic error Memory leak auth2.i 11503 6 View
<http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-ABKOwr.html#EndPath>


http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-ABKOwr.html#EndPath

fakepw() returns a static variable.

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


keisial at gmail

Feb 5, 2012, 2:04 PM

Post #11 of 19 (1382 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
On 04/02/12 02:55, Zhenbo Xu wrote:
> The url below is the index of bug reports that are checked as real
> bugs manually.
>
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/index.html
>
>
> Shall we fix these bugs? Or just let them go since they are not so serious?
>
> Hope for your replies!
The 10th report is another false positive:
Logic error Memory leak auth-options.i 10587 28 View Report
<http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-mVEeJj.html#EndPath>


http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-mVEeJj.html#EndPath

Melton complains that in line 10587 the memory of data wasn't released,
but there's a call to buffer_free(&data);
in line 10585.


_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


keisial at gmail

Feb 5, 2012, 2:07 PM

Post #12 of 19 (1383 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
On 05/02/12 23:04, 聲gel Gonz嫮ez wrote:
> The 10th report is another false positive:
> Logic error Memory leak auth-options.i 10587 28 View Report
> <http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-mVEeJj.html#EndPath>
>
>
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-mVEeJj.html#EndPath
>
> Melton complains that in line 10587 the memory of data wasn't released,
> but there's a call to buffer_free(&data);
> in line 10585.

Same false positive for 11th, 13th & 14th.
http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-UDrpYF.html#EndPath
http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-BtfghY.html#EndPath
http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-XXMHNK.html#EndPath


_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


zhenbo1987 at gmail

Feb 5, 2012, 5:53 PM

Post #13 of 19 (1382 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
2012/2/6 聲gel Gonz嫮ez <keisial [at] gmail>

> On 04/02/12 02:55, Zhenbo Xu wrote:
>
> I also applied melton(http://lcs.ios.ac.cn/~xuzb/melton.html)
>
> to detect the potential bugs in sshd (openssh-5.9p1).
>
>
> The url below is the index of bug reports that are checked as real
> bugs manually.
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/index.html
>
>
> Shall we fix these bugs? Or just let them go since they are not so serious?
> Hope for your replies!
>
>
> The second leak
>
> Logic error Memory leak session.i 13193 13 View Report<http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-zSMfqI.html#EndPath>
>
>
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-zSMfqI.html#EndPath
>
> is a false positive.
>
> The only way to exit the infinite loop is the return of line 13267.
> And line 13266 calls session_close(), which frees s->auth_data in line
> 14994.
> (granted, it's tricky to follow...)
>

This report may not be so clear.
The leak occurs at the second loop iteration when the "type =
packet_read()" is equal to case 34 (line 13192) twice.
At the first iteration, s->auth_proto gets a heap buffer but is not freed.
And at the second iteration, s->auth_proto is reassigned,
which means the heap buffer allocated at the first iteration leaks.


> The analysis seem to have stopped in line 13193, after step 14.
>
> As a suggestion, I recommend you to make the messages "Execution continues
> on line 12345"
> links to line 12345.
> Also, going to the opening brace from the closing one would be useful when
> dealing with big blocks.
>
>
Thank you for your suggestion and your detail replies!



--
Zhenbo Xu
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


zhenbo1987 at gmail

Feb 5, 2012, 6:03 PM

Post #14 of 19 (1387 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
2012/2/6 聲gel Gonz嫮ez <keisial [at] gmail>

> On 04/02/12 02:55, Zhenbo Xu wrote:
>
> The url below is the index of bug reports that are checked as real
> bugs manually.
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/index.html
>
>
> Shall we fix these bugs? Or just let them go since they are not so serious?
>
> Hope for your replies!
>
> The 8th report is another false positive
> Logic error Memory leak auth2.i 11503 6 View<http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-ABKOwr.html#EndPath>
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-ABKOwr.html#EndPath
>
> fakepw() returns a static variable.
>
> Shall we free authctxt->pw before this assignment authctxt->pw =
fakepw();,
Since authctxt->pw gets a heap space at 11496. (authctxt->pw =
(use_privsep ? mm_getpwnamallow(user) :
getpwnamallow(user)<http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/linked_files/linked-oH0nvi.html#Path7_2>
);)



--
Zhenbo Xu
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


zhenbo1987 at gmail

Feb 5, 2012, 6:30 PM

Post #15 of 19 (1381 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
2012/2/6 聲gel Gonz嫮ez <keisial [at] gmail>

> On 04/02/12 02:55, Zhenbo Xu wrote:
>
> The url below is the index of bug reports that are checked as real
> bugs manually.
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/index.html
>
>
> Shall we fix these bugs? Or just let them go since they are not so serious?
>
> Hope for your replies!
>
> The 10th report is another false positive:
> Logic error Memory leak auth-options.i 10587 28 View Report<http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-mVEeJj.html#EndPath>
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-mVEeJj.html#EndPath
>
> Melton complains that in line 10587 the memory of data wasn't released,
> but there's a call to buffer_free(&data);
> in line 10585.
>
>
What melton complains is the heap object returned by
buffer_get_cstring_ret<http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/linked_files/linked-TOML0p.html#Path29_2>
wasn't
released.




--
Zhenbo Xu
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


zhenbo1987 at gmail

Feb 5, 2012, 6:33 PM

Post #16 of 19 (1385 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
Same with these reports.

2012/2/6 聲gel Gonz嫮ez <keisial [at] gmail>

> On 05/02/12 23:04, 聲gel Gonz嫮ez wrote:
> > The 10th report is another false positive:
> > Logic error Memory leak auth-options.i 10587 28 View Report
> > <
> http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-mVEeJj.html#EndPath
> >
> >
> >
> >
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-mVEeJj.html#EndPath
> >
> > Melton complains that in line 10587 the memory of data wasn't released,
> > but there's a call to buffer_free(&data);
> > in line 10585.
>
> Same false positive for 11th, 13th & 14th.
>
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-UDrpYF.html#EndPath
>
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-BtfghY.html#EndPath
>
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-XXMHNK.html#EndPath
>
>
>


--
Zhenbo Xu
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


keisial at gmail

Feb 6, 2012, 12:58 AM

Post #17 of 19 (1364 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
On 06/02/12 03:30, Zhenbo Xu wrote:
>
> The 10th report is another false positive:
> Logic error Memory leak auth-options.i 10587 28 View Report
> <http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-mVEeJj.html#EndPath>
>
>
> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-mVEeJj.html#EndPath
> <http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-mVEeJj.html#EndPath>
>
> Melton complains that in line 10587 the memory of data wasn't
> released, but there's a call to buffer_free(&data);
> in line 10585.
>
>
> What melton complains is the heap object returned
> by buffer_get_cstring_ret
> <http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/linked_files/linked-TOML0p.html#Path29_2> wasn't
> released.
You are right. I missed that it was about allowed.
However, Melton is doing:

> switch (addr_match_cidr_list(remote_ip,
>
>
> 11
> Control jumps to 'case 1:' at line 10525
>
> 10524 allowed)) {
> 10525 case 1:
> 10526
> 10527 xfree(allowed);
> 10528 break;
>
>
> 12
> Execution continues on line 10548
>
Where it is freed (and add_match_cidr_list can only return -1, 0 or 1 so
all cases are covered).

Then it seems to do another loop where buffer_get_cstring_ret
<http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/linked_files/linked-TOML0p.html#Path29_2>
returns NULL, so nothing was allocated.

I still don't see the leak.


_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


zhenbo1987 at gmail

Feb 6, 2012, 1:09 AM

Post #18 of 19 (1369 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
2012/2/6 聲gel Gonz嫮ez <keisial [at] gmail>

> On 06/02/12 03:30, Zhenbo Xu wrote:
>
> The 10th report is another false positive:
>> Logic error Memory leak auth-options.i 10587 28 View Report<http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-mVEeJj.html#EndPath>
>> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-mVEeJj.html#EndPath
>>
>> Melton complains that in line 10587 the memory of data wasn't released,
>> but there's a call to buffer_free(&data);
>> in line 10585.
>>
>>
> What melton complains is the heap object returned by
> buffer_get_cstring_ret<http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/linked_files/linked-TOML0p.html#Path29_2> wasn't
> released.
>
> You are right. I missed that it was about allowed.
> However, Melton is doing:
>
> switch (addr_match_cidr_list(remote_ip,
> 11
> Control jumps to 'case 1:' at line 10525
> 10524 allowed)) { 10525 case 1: 10526 10527 xfree(allowed); 10528
> break;
> 12
> Execution continues on line 10548
>
> Where it is freed (and add_match_cidr_list can only return -1, 0 or 1 so
> all cases are covered).
>
> Then it seems to do another loop where buffer_get_cstring_ret<http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/linked_files/linked-TOML0p.html#Path29_2>returns NULL, so nothing was allocated.
>
> I still don't see the leak.
>
>
What if line 10511 takes the true branch, and it just "goto out"in line
10514 without releasing allowed.



--
Zhenbo Xu
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev


apb at cequrux

Feb 6, 2012, 3:26 AM

Post #19 of 19 (1374 views)
Permalink
Re: Potential memory leak in sshd [detected by melton] [In reply to]
On Mon, 06 Feb 2012, ngel Gonzlez wrote:
>> The 10th report is another false positive:
>> Logic error Memory leak auth-options.i 10587 28 View Report
>> <http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-mVEeJj.html#EndPath>
>>
>>
>> http://lcs.ios.ac.cn/~xuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-mVEeJj.html#EndPath
>> <http://lcs.ios.ac.cn/%7Exuzb/bugsfound/memleak/openssh-5.9p1/realbugs/sshd/report-mVEeJj.html#EndPath>
>>
>> Melton complains that in line 10587 the memory of data wasn't
>> released, but there's a call to buffer_free(&data);
>> in line 10585.

Similarly to several of the other cases, this leak occurs only
with unusual input. If "source-address" appears twice in the
options, then the allocation in line 10505 occurs twice; the first
allocation leaks, and the second allocation is freed later.

The presence of both "7 Taking true branch" and "21 Taking true
branch" at line 10504 is the clue that this leak occurs when
"source-address" appears twice.

--apb (Alan Barrett)
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

OpenSSH dev RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.