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