Bug 2883 - Coverity issues
Summary: Coverity issues
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: 7.7p1
Hardware: Other Linux
: P5 enhancement
Assignee: Damien Miller
URL:
Keywords:
Depends on:
Blocks: V_7_8
  Show dependency treegraph
 
Reported: 2018-07-19 00:46 AEST by Jakub Jelen
Modified: 2023-01-13 13:41 AEDT (History)
2 users (show)

See Also:


Attachments
resolve memory leaks (7.28 KB, text/plain)
2018-07-19 00:46 AEST, Jakub Jelen
no flags Details
tweaked diff (9.57 KB, patch)
2018-07-27 14:02 AEST, Damien Miller
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelen 2018-07-19 00:46:44 AEST
Created attachment 3164 [details]
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.
Comment 1 Damien Miller 2018-07-27 14:02:45 AEST
Created attachment 3166 [details]
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.
Comment 2 Darren Tucker 2018-07-27 14:19:44 AEST
Comment on attachment 3166 [details]
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
Comment 3 Jakub Jelen 2018-07-30 19:24:25 AEST
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.
Comment 4 Damien Miller 2018-08-02 16:11:49 AEST
This has been committed in 74287f5df99 and will be in openssh-7.8; thanks!
Comment 5 Damien Miller 2021-04-23 15:01:31 AEST
closing resolved bugs as of 8.6p1 release