URL: https://github.com/SSSD/sssd/pull/916
Author: alexey-tikhonov
Title: #916: Fix one coverity issue
Action: synchronized
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 13f17d7bd93ba6d676383ea89c3d405c7842a148 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..445ab9fc6f 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(sock);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, "Failed to make socket nonblocking\n");
goto done;
}
From 9af728b2302ba863b0f1c6df3402f43526ca542c 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 445ab9fc6f..102f5622c2 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(sock);
if (ret != EOK) {
From 09fde270e885db37d559be86d8fc2776677afeaa Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikh...@redhat.com>
Date: Tue, 5 Nov 2019 15:35:35 +0100
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 | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/sss_client/ssh/sss_ssh_knownhostsproxy.c b/src/sss_client/ssh/sss_ssh_knownhostsproxy.c
index 102f5622c2..051f51c382 100644
--- a/src/sss_client/ssh/sss_ssh_knownhostsproxy.c
+++ b/src/sss_client/ssh/sss_ssh_knownhostsproxy.c
@@ -63,10 +63,14 @@ connect_socket(int family, struct sockaddr *addr, size_t addr_len, int *sd)
goto done;
}
- *sd = sock;
-
done:
- if (ret != 0 && sock >= 0) close(sock);
+ if (ret != 0) {
+ if (sock >= 0) {
+ close(sock);
+ }
+ } else {
+ *sd = sock;
+ }
return ret;
}
_______________________________________________
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
