URL: https://github.com/SSSD/sssd/pull/916
Author: alexey-tikhonov
Title: #916: Fix one coverity issue
Action: opened
PR body:
"""
...and reduce code duplication a little bit.
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/916/head:pr916
git checkout pr916
From 4b25d831fddb00f0bd32d06b8cd3c0ae784e767f Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikh...@redhat.com>
Date: Thu, 24 Oct 2019 12:19:45 +0200
Subject: [PATCH 1/3] Reduces code duplication
This patch makes use of existing sss_fd_nonblocking() function where
applicable to reduce code duplication.
---
src/monitor/monitor_netlink.c | 9 ++---
src/sss_client/ssh/sss_ssh_knownhostsproxy.c | 36 ++++----------------
2 files changed, 9 insertions(+), 36 deletions(-)
diff --git a/src/monitor/monitor_netlink.c b/src/monitor/monitor_netlink.c
index a54ae5ad6d..3703a148dc 100644
--- a/src/monitor/monitor_netlink.c
+++ b/src/monitor/monitor_netlink.c
@@ -791,7 +791,6 @@ int setup_netlink(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
struct netlink_ctx *nlctx;
int ret;
int nlfd;
- unsigned flags;
int groups[] = { RTNLGRP_LINK, RTNLGRP_IPV4_ROUTE, RTNLGRP_IPV6_ROUTE,
RTNLGRP_IPV4_IFADDR, RTNLGRP_IPV6_IFADDR, 0 };
@@ -847,12 +846,8 @@ int setup_netlink(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
nlw_disable_seq_check(nlctx->nlp);
nlfd = nl_socket_get_fd(nlctx->nlp);
- flags = fcntl(nlfd, F_GETFL, 0);
-
- errno = 0;
- ret = fcntl(nlfd, F_SETFL, flags | O_NONBLOCK);
- if (ret < 0) {
- ret = errno;
+ ret = sss_fd_nonblocking(nlfd);
+ if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
"Cannot set the netlink fd to nonblocking\n");
goto fail;
diff --git a/src/sss_client/ssh/sss_ssh_knownhostsproxy.c b/src/sss_client/ssh/sss_ssh_knownhostsproxy.c
index c714506323..34e0a53781 100644
--- a/src/sss_client/ssh/sss_ssh_knownhostsproxy.c
+++ b/src/sss_client/ssh/sss_ssh_knownhostsproxy.c
@@ -42,25 +42,14 @@
static int
connect_socket(int family, struct sockaddr *addr, size_t addr_len, int *sd)
{
- int flags;
int sock = -1;
int ret;
/* set O_NONBLOCK on standard input */
- flags = fcntl(0, F_GETFL);
- if (flags == -1) {
- ret = errno;
- DEBUG(SSSDBG_OP_FAILURE, "fcntl() failed (%d): %s\n",
- ret, strerror(ret));
- goto done;
- }
-
- ret = fcntl(0, F_SETFL, flags | O_NONBLOCK);
- if (ret == -1) {
- ret = errno;
- DEBUG(SSSDBG_OP_FAILURE, "fcntl() failed (%d): %s\n",
- ret, strerror(ret));
- goto done;
+ ret = sss_fd_nonblocking(0);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, "Failed to make fd=0 nonblocking\n");
+ return ret;
}
/* create socket */
@@ -90,7 +79,6 @@ connect_socket(int family, struct sockaddr *addr, size_t addr_len, int *sd)
static int proxy_data(int sock)
{
- int flags;
struct pollfd fds[2];
char buffer[BUFFER_SIZE];
int i;
@@ -98,19 +86,9 @@ static int proxy_data(int sock)
int ret;
/* set O_NONBLOCK on the socket */
- flags = fcntl(sock, F_GETFL);
- if (flags == -1) {
- ret = errno;
- DEBUG(SSSDBG_OP_FAILURE, "fcntl() failed (%d): %s\n",
- ret, strerror(ret));
- goto done;
- }
-
- ret = fcntl(sock, F_SETFL, flags | O_NONBLOCK);
- if (ret == -1) {
- ret = errno;
- DEBUG(SSSDBG_OP_FAILURE, "fcntl() failed (%d): %s\n",
- ret, strerror(ret));
+ ret = sss_fd_nonblocking(0);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, "Failed to make socket nonblocking\n");
goto done;
}
From 6570a1f9223c442180a56c7bc24717ae71d2794f Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikh...@redhat.com>
Date: Thu, 24 Oct 2019 12:44:31 +0200
Subject: [PATCH 2/3] sss_ssh_knownhostsproxy: relocated O_NONBLOCK setting
Relocated sss_fd_nonblocking(0) to proxy_data() from connect_socket()
as logically it makes more sense and avoids redundant operations in case
connect_socket() is called several times in a loop.
---
src/sss_client/ssh/sss_ssh_knownhostsproxy.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/sss_client/ssh/sss_ssh_knownhostsproxy.c b/src/sss_client/ssh/sss_ssh_knownhostsproxy.c
index 34e0a53781..d13e7a32da 100644
--- a/src/sss_client/ssh/sss_ssh_knownhostsproxy.c
+++ b/src/sss_client/ssh/sss_ssh_knownhostsproxy.c
@@ -45,13 +45,6 @@ connect_socket(int family, struct sockaddr *addr, size_t addr_len, int *sd)
int sock = -1;
int ret;
- /* set O_NONBLOCK on standard input */
- ret = sss_fd_nonblocking(0);
- if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE, "Failed to make fd=0 nonblocking\n");
- return ret;
- }
-
/* create socket */
sock = socket(family, SOCK_STREAM, IPPROTO_TCP);
if (sock == -1) {
@@ -85,6 +78,13 @@ static int proxy_data(int sock)
ssize_t res;
int ret;
+ /* set O_NONBLOCK on standard input */
+ ret = sss_fd_nonblocking(0);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, "Failed to make fd=0 nonblocking\n");
+ goto done;
+ }
+
/* set O_NONBLOCK on the socket */
ret = sss_fd_nonblocking(0);
if (ret != EOK) {
From 5e20fbd5e9adddfc635fdf6e4046bd692097ac4c Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikh...@redhat.com>
Date: Thu, 24 Oct 2019 12:53:56 +0200
Subject: [PATCH 3/3] sss_ssh_knownhostsproxy: fixed Coverity issue
Actually I think this Coverity error was "false positive":
```
Error: RESOURCE_LEAK (CWE-772):
sssd-2.2.3/src/sss_client/ssh/sss_ssh_knownhostsproxy.c:67: open_fn: Returning handle opened by "socket".
sssd-2.2.3/src/sss_client/ssh/sss_ssh_knownhostsproxy.c:67: var_assign: Assigning: "sock" = handle returned from "socket(family, SOCK_STREAM, IPPROTO_TCP)".
sssd-2.2.3/src/sss_client/ssh/sss_ssh_knownhostsproxy.c:76: noescape: Resource "sock" is not freed or pointed-to in "connect".
sssd-2.2.3/src/sss_client/ssh/sss_ssh_knownhostsproxy.c:88: leaked_handle: Handle variable "sock" going out of scope leaks the handle.
86| done:
87| if (ret != 0 && sock >= 0) close(sock);
88|-> return ret;
89| }
90|
```
Nonetheless it is easier to adjust the code to avoid a complaint.
---
src/sss_client/ssh/sss_ssh_knownhostsproxy.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/sss_client/ssh/sss_ssh_knownhostsproxy.c b/src/sss_client/ssh/sss_ssh_knownhostsproxy.c
index d13e7a32da..f8f7c6f0a8 100644
--- a/src/sss_client/ssh/sss_ssh_knownhostsproxy.c
+++ b/src/sss_client/ssh/sss_ssh_knownhostsproxy.c
@@ -51,23 +51,22 @@ connect_socket(int family, struct sockaddr *addr, size_t addr_len, int *sd)
ret = errno;
DEBUG(SSSDBG_OP_FAILURE, "socket() failed (%d): %s\n",
ret, strerror(ret));
- goto done;
+ return ret;
}
/* connect to the server */
ret = connect(sock, addr, addr_len);
if (ret == -1) {
ret = errno;
+ close(sock);
DEBUG(SSSDBG_OP_FAILURE, "connect() failed (%d): %s\n",
ret, strerror(ret));
- goto done;
+ return ret;
}
*sd = sock;
-done:
- if (ret != 0 && sock >= 0) close(sock);
- return ret;
+ return EOK;
}
static int proxy_data(int sock)
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
