bugzilla-daemon at bugzilla.mindrot.org
2018-Jul-18 14:46 UTC
[Bug 2883] New: Coverity issues
https://bugzilla.mindrot.org/show_bug.cgi?id=2883 Bug ID: 2883 Summary: Coverity issues Product: Portable OpenSSH Version: 7.7p1 Hardware: Other OS: Linux Status: NEW Severity: enhancement Priority: P5 Component: ssh Assignee: unassigned-bugs at mindrot.org Reporter: jjelen at redhat.com Created attachment 3164 --> https://bugzilla.mindrot.org/attachment.cgi?id=3164&action=edit resolve memory leaks Hello, we ran the coverity scan on openssh-7.7p1 I noticed few memory leaks (see attached patch-set). This is nothing urgent, but it would be nice to have clean memory footprint. -- You are receiving this mail because: You are watching the assignee of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=2883 Damien Miller <djm at mindrot.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |djm at mindrot.org Attachment #3164|text/x-log |text/plain mime type| | -- You are receiving this mail because: You are watching the assignee of the bug. You are watching someone on the CC list of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=2883 Damien Miller <djm at mindrot.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dtucker at dtucker.net Assignee|unassigned-bugs at mindrot.org |djm at mindrot.org Status|NEW |ASSIGNED Attachment #3166| |ok?(dtucker at dtucker.net) Flags| | --- Comment #1 from Damien Miller <djm at mindrot.org> --- Created attachment 3166 --> https://bugzilla.mindrot.org/attachment.cgi?id=3166&action=edit tweaked diff Here's a revised diff. Most of the changes are just stylistic and refactoring, but I think the original sftp-client.c upload_dir_internal() change had a bug: it was checking sb before the lstat() call that filled it. -- You are receiving this mail because: You are watching someone on the CC list of the bug. You are watching the assignee of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=2883 Damien Miller <djm at mindrot.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |2852 Referenced Bugs: https://bugzilla.mindrot.org/show_bug.cgi?id=2852 [Bug 2852] Tracking bug for OpenSSH 7.8 release -- You are receiving this mail because: You are watching the assignee of the bug. You are watching someone on the CC list of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=2883 --- Comment #2 from Darren Tucker <dtucker at dtucker.net> --- Comment on attachment 3166 --> https://bugzilla.mindrot.org/attachment.cgi?id=3166 tweaked diff>- struct addrinfo hints, *ai; >+ struct addrinfo hints, *ai = NULL;[...]>+ out: > freeaddrinfo(ai);I don't think there's any guarantee freeaddrinfo(NULL) is safe. I don't see it in the man page for openbsd or linux and I don't see it in rfc3493 -- You are receiving this mail because: You are watching someone on the CC list of the bug. You are watching the assignee of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=2883 --- Comment #3 from Jakub Jelen <jjelen at redhat.com> --- Darren, you are right. There should be NULL check before calling the freeaddrinfo() or just return before the addrinfo is allocated, splitting the conditions. Damien, you are right about the sftp. You solution looks more elegant. Thanks. -- You are receiving this mail because: You are watching the assignee of the bug. You are watching someone on the CC list of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=2883 --- Comment #4 from Damien Miller <djm at mindrot.org> --- This has been committed in 74287f5df99 and will be in openssh-7.8; thanks! -- You are receiving this mail because: You are watching someone on the CC list of the bug. You are watching the assignee of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=2883 Damien Miller <djm at mindrot.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|ASSIGNED |RESOLVED -- You are receiving this mail because: You are watching someone on the CC list of the bug. You are watching the assignee of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=2883 Damien Miller <djm at mindrot.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #5 from Damien Miller <djm at mindrot.org> --- closing resolved bugs as of 8.6p1 release -- You are receiving this mail because: You are watching someone on the CC list of the bug. You are watching the assignee of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=2883 Damien Miller <djm at mindrot.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #3166|ok?(dtucker at dtucker.net) | Flags| | -- You are receiving this mail because: You are watching the assignee of the bug. You are watching someone on the CC list of the bug.