Fix flags handling in gss_init_sec_context()

resolves: https://fedorahosted.org/gss-proxy/ticket/112
Fix nfsd startup
resolves: https://fedorahosted.org/gss-proxy/ticket/114
Fix potential mutex deadlock
resolves: https://fedorahosted.org/gss-proxy/ticket/120
Fix segfault in gssi_inquire_context
resolves: https://fedorahosted.org/gss-proxy/ticket/117
resolves: #1061133
This commit is contained in:
Günther Deschner 2014-03-13 16:46:46 +01:00
parent a3731ae64d
commit 5346047bd4
5 changed files with 367 additions and 1 deletions

View File

@ -0,0 +1,36 @@
From f39b471f34b381784a1bd1906bf8335ac2c7ef5e Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Tue, 11 Mar 2014 18:16:32 -0400
Subject: [PATCH] Properly cleanup mutex on failure.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
If the call to create socket fails we leave a dangling lock and the client
enters into a deadlock on the next call.
Fixes: https://fedorahosted.org/gss-proxy/ticket/121
Signed-off-by: Simo Sorce <simo@redhat.com>
Reviewed-by: Günther Deschner <gdeschner@redhat.com>
---
proxy/src/client/gpm_common.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c
index 74296da..4651194 100644
--- a/proxy/src/client/gpm_common.c
+++ b/proxy/src/client/gpm_common.c
@@ -153,6 +153,9 @@ static int gpm_grab_sock(struct gpm_ctx *gpmctx)
ret = gpm_open_socket(gpmctx);
}
+ if (ret) {
+ pthread_mutex_unlock(&gpmctx->lock);
+ }
return ret;
}
--
1.8.5.3

View File

@ -0,0 +1,31 @@
From 8b147c9196d9068d0fc5e5a8919b84e8cbb97ef4 Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Fri, 6 Dec 2013 17:51:14 -0500
Subject: [PATCH] Fix config token parsing.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Resolves: https://fedorahosted.org/gss-proxy/ticket/112
Signed-off-by: Simo Sorce <simo@redhat.com>
Reviewed-by: Günther Deschner <gdeschner@redhat.com>
---
proxy/src/gp_config.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/proxy/src/gp_config.c b/proxy/src/gp_config.c
index 2fc4a6f..ee96975 100644
--- a/proxy/src/gp_config.c
+++ b/proxy/src/gp_config.c
@@ -153,7 +153,6 @@ static int parse_flags(const char *value, uint32_t *storage)
return ENOMEM;
}
- token = strtok_r(str, ", ", &handle);
for (token = strtok_r(str, ", ", &handle);
token != NULL;
token = strtok_r(NULL, ", ", &handle)) {
--
1.8.3.1

View File

@ -0,0 +1,39 @@
From c17f20b949d2e80e596ce21ecd944db80aaa80b1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=BCnther=20Deschner?= <gdeschner@redhat.com>
Date: Wed, 29 Jan 2014 17:59:03 +0100
Subject: [PATCH] Fix potential segfault in gssi_inquire_context().
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Günther Deschner <gdeschner@redhat.com>
Reviewed-by: Simo Sorce <simo@redhat.com>
---
proxy/src/mechglue/gpp_context.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/proxy/src/mechglue/gpp_context.c b/proxy/src/mechglue/gpp_context.c
index 6010724..bb16a93 100644
--- a/proxy/src/mechglue/gpp_context.c
+++ b/proxy/src/mechglue/gpp_context.c
@@ -223,7 +223,7 @@ OM_uint32 gssi_inquire_context(OM_uint32 *minor_status,
maj = gss_inquire_context(&min,
ctx_handle->local,
s_name ? &s_name->local : NULL,
- s_name ? &t_name->local : NULL,
+ t_name ? &t_name->local : NULL,
lifetime_rec,
&mech_oid,
ctx_flags,
@@ -233,7 +233,7 @@ OM_uint32 gssi_inquire_context(OM_uint32 *minor_status,
maj = gpm_inquire_context(&min,
ctx_handle->remote,
s_name ? &s_name->remote : NULL,
- s_name ? &t_name->remote : NULL,
+ t_name ? &t_name->remote : NULL,
lifetime_rec,
&mech_oid,
ctx_flags,
--
1.8.5.3

View File

@ -0,0 +1,240 @@
From 58a39677c961c72b052eae0b9d94b992254d6e10 Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Fri, 3 Jan 2014 16:45:35 -0500
Subject: [PATCH 1/2] Add utility functions to read()/write() safely
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Automatically handle short reads due to singals interrupting the process.
Signed-off-by: Simo Sorce <simo@redhat.com>
Reviewed-by: Günther Deschner <gdeschner@redhat.com>
---
proxy/src/gp_common.h | 2 ++
proxy/src/gp_util.c | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 41 insertions(+)
diff --git a/proxy/src/gp_common.h b/proxy/src/gp_common.h
index f2b8c3e..3a1b7be 100644
--- a/proxy/src/gp_common.h
+++ b/proxy/src/gp_common.h
@@ -69,6 +69,8 @@ bool gp_same(const char *a, const char *b);
bool gp_boolean_is_true(const char *s);
char *gp_getenv(const char *name);
+ssize_t gp_safe_read(int fd, void *buf, size_t count);
+ssize_t gp_safe_write(int fd, const void *buf, size_t count);
/* NOTE: read the note in gp_util.c before using gp_strerror() */
char *gp_strerror(int errnum);
diff --git a/proxy/src/gp_util.c b/proxy/src/gp_util.c
index 4fbac4e..34f3024 100644
--- a/proxy/src/gp_util.c
+++ b/proxy/src/gp_util.c
@@ -29,6 +29,7 @@
#include <stdlib.h>
#include <stdio.h>
#include <errno.h>
+#include <unistd.h>
bool gp_same(const char *a, const char *b)
{
@@ -125,3 +126,41 @@ char *gp_strerror(int errnum)
errno = saved_errno;
return buf;
}
+
+ssize_t gp_safe_read(int fd, void *buf, size_t count)
+{
+ char *b = (char *)buf;
+ ssize_t len = 0;
+ ssize_t ret;
+
+ do {
+ ret = read(fd, &b[len], count - len);
+ if (ret == -1) {
+ if (errno == EINTR) continue;
+ return ret;
+ }
+ if (ret == 0) break; /* EOF */
+ len += ret;
+ } while (count > len);
+
+ return len;
+}
+
+ssize_t gp_safe_write(int fd, const void *buf, size_t count)
+{
+ const char *b = (const char *)buf;
+ ssize_t len = 0;
+ ssize_t ret;
+
+ do {
+ ret = write(fd, &b[len], count - len);
+ if (ret == -1) {
+ if (errno == EINTR) continue;
+ return ret;
+ }
+ if (ret == 0) break; /* EOF */
+ len += ret;
+ } while (count > len);
+
+ return len;
+}
--
1.8.4.2
From bd8ffcf67be8fdbe14bc49a65a8eafe904119d88 Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Fri, 3 Jan 2014 12:10:36 -0500
Subject: [PATCH 2/2] Block parent process until child is initialized.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This way the init system will not proceed starting dependencies until gssproxy
is actually ready to serve requests.
In particular this is used to make sure the nfsd proc file has been touched
before the nfsd server is started.
Resolves: https://fedorahosted.org/gss-proxy/ticket/114
Signed-off-by: Simo Sorce <simo@redhat.com>
Reviewed-by: Günther Deschner <gdeschner@redhat.com>
---
proxy/src/gp_init.c | 42 +++++++++++++++++++++++++++++++++++++++---
proxy/src/gp_proxy.h | 3 ++-
proxy/src/gssproxy.c | 11 +++++++++--
3 files changed, 50 insertions(+), 6 deletions(-)
diff --git a/proxy/src/gp_init.c b/proxy/src/gp_init.c
index 830ae16..6207a78 100644
--- a/proxy/src/gp_init.c
+++ b/proxy/src/gp_init.c
@@ -37,12 +37,22 @@
#include <stdio.h>
#include "gp_proxy.h"
-void init_server(bool daemonize)
+void init_server(bool daemonize, int *wait_fd)
{
pid_t pid, sid;
int ret;
+ *wait_fd = -1;
+
if (daemonize) {
+ int pipefd[2];
+ char buf[1];
+
+ /* create parent-child pipe */
+ ret = pipe(pipefd);
+ if (ret == -1) {
+ exit(EXIT_FAILURE);
+ }
pid = fork();
if (pid == -1) {
@@ -50,10 +60,22 @@ void init_server(bool daemonize)
exit(EXIT_FAILURE);
}
if (pid != 0) {
- /* ok kill the parent */
- exit(EXIT_SUCCESS);
+ /* wait for child to signal it is ready */
+ close(pipefd[1]);
+ ret = gp_safe_read(pipefd[0], buf, 1);
+ if (ret == 1) {
+ /* child signaled all ok */
+ exit(EXIT_SUCCESS);
+ } else {
+ /* lost child, something went wrong */
+ exit(EXIT_FAILURE);
+ }
}
+ /* child */
+ close(pipefd[0]);
+ *wait_fd = pipefd[1];
+
sid = setsid();
if (sid == -1) {
/* setsid error ? abort */
@@ -78,6 +100,20 @@ void init_server(bool daemonize)
gp_logging_init();
}
+void init_done(int wait_fd)
+{
+ char buf = 0;
+ int ret;
+
+ if (wait_fd != -1) {
+ ret = gp_safe_write(wait_fd, &buf, 1);
+ if (ret != 1) {
+ exit(EXIT_FAILURE);
+ }
+ close(wait_fd);
+ }
+}
+
void fini_server(void)
{
closelog();
diff --git a/proxy/src/gp_proxy.h b/proxy/src/gp_proxy.h
index 733fec5..79bebb8 100644
--- a/proxy/src/gp_proxy.h
+++ b/proxy/src/gp_proxy.h
@@ -106,7 +106,8 @@ struct gp_creds_handle *gp_service_get_creds_handle(struct gp_service *svc);
void free_config(struct gp_config **config);
/* from gp_init.c */
-void init_server(bool daemonize);
+void init_server(bool daemonize, int *wait_fd);
+void init_done(int wait_fd);
void fini_server(void);
verto_ctx *init_event_loop(void);
void init_proc_nfsd(struct gp_config *cfg);
diff --git a/proxy/src/gssproxy.c b/proxy/src/gssproxy.c
index 1bf0a0b..80430d6 100644
--- a/proxy/src/gssproxy.c
+++ b/proxy/src/gssproxy.c
@@ -42,6 +42,7 @@ int main(int argc, const char *argv[])
int vflags;
struct gssproxy_ctx *gpctx;
struct gp_sock_ctx *sock_ctx;
+ int wait_fd;
int ret;
int i;
@@ -97,7 +98,7 @@ int main(int argc, const char *argv[])
exit(EXIT_FAILURE);
}
- init_server(gpctx->config->daemonize);
+ init_server(gpctx->config->daemonize, &wait_fd);
write_pid();
@@ -139,9 +140,15 @@ int main(int argc, const char *argv[])
}
}
- /* special call to tell the Linux kernel gss-proxy is available */
+ /* We need to tell nfsd that GSS-Proxy is available before it starts,
+ * as nfsd needs to know GSS-Proxy is in use before the first time it
+ * needs to call accept_sec_context. */
init_proc_nfsd(gpctx->config);
+ /* Now it is safe to tell the init system that we're done starting up,
+ * so it can continue with dependencies and start nfsd */
+ init_done(wait_fd);
+
ret = gp_workers_init(gpctx);
if (ret) {
exit(EXIT_FAILURE);
--
1.8.4.2

View File

@ -1,6 +1,6 @@
Name: gssproxy
Version: 0.3.1
Release: 0%{?dist}
Release: 1%{?dist}
Summary: GSSAPI Proxy
Group: System Environment/Libraries
@ -9,6 +9,11 @@ URL: http://fedorahosted.org/gss-proxy
Source0: http://fedorahosted.org/released/gss-proxy/%{name}-%{version}.tar.gz
BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
Patch0: gssproxy-0.3.1-flags_handling.patch
Patch1: gssproxy-0.3.1-nfsd_startup.patch
Patch2: gssproxy-0.3.1-deadlock_fix.patch
Patch3: gssproxy-0.3.1-gssi_inquire_context.patch
%global servicename gssproxy
%global pubconfpath %{_sysconfdir}/gssproxy
%global gpstatedir %{_localstatedir}/lib/gssproxy
@ -52,6 +57,10 @@ A proxy for GSSAPI credential handling
%prep
%setup -q
%patch0 -p2 -b .flags_handling
%patch1 -p2 -b .nfsd_startup
%patch2 -p2 -b .deadlock_fix
%patch3 -p2 -b .gssi_inquire_context
%build
autoreconf -f -i
@ -104,6 +113,17 @@ rm -rf %{buildroot}
%systemd_postun_with_restart gssproxy.service
%changelog
* Thu Mar 13 2014 Guenther Deschner <gdeschner@redhat.com> 0.3.1-1
- Fix flags handling in gss_init_sec_context()
- resolves: https://fedorahosted.org/gss-proxy/ticket/112
- Fix nfsd startup
- resolves: https://fedorahosted.org/gss-proxy/ticket/114
- Fix potential mutex deadlock
- resolves: https://fedorahosted.org/gss-proxy/ticket/120
- Fix segfault in gssi_inquire_context
- resolves: https://fedorahosted.org/gss-proxy/ticket/117
- resolves: #1061133
* Tue Nov 26 2013 Guenther Deschner <gdeschner@redhat.com> 0.3.1-0
- New upstream release 0.3.1:
* Fix use of gssproxy for client initiation