| Message ID | 1384542931-18753-3-git-send-email-jlayton@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Nov 15, 2013 at 02:15:31PM -0500, Jeff Layton wrote: > With some proposed kernel changes, it won't even attempt to upcall > sometimes if it doesn't appear that gssd is running. This means that > we have a theoretical race between gssd starting up at boot time and > the init process attempting to mount kerberized filesystems. > > Fix this by switching gssd to use mydaemon() and having the child > only release the parent after it has processed the directory once. Makes sense to me, thanks--ACK. --b. > > Signed-off-by: Jeff Layton <jlayton@redhat.com> > --- > utils/gssd/gssd.c | 6 ++++-- > utils/gssd/gssd.h | 1 + > utils/gssd/gssd_main_loop.c | 4 ++++ > 3 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c > index 8ee478b..fdad153 100644 > --- a/utils/gssd/gssd.c > +++ b/utils/gssd/gssd.c > @@ -54,6 +54,7 @@ > #include "err_util.h" > #include "gss_util.h" > #include "krb5_util.h" > +#include "nfslib.h" > > char pipefs_dir[PATH_MAX] = GSSD_PIPEFS_DIR; > char keytabfile[PATH_MAX] = GSSD_DEFAULT_KEYTAB_FILE; > @@ -63,6 +64,7 @@ int use_memcache = 0; > int root_uses_machine_creds = 1; > unsigned int context_timeout = 0; > char *preferred_realm = NULL; > +int pipefds[2] = { -1, -1 }; > > void > sig_die(int signal) > @@ -187,8 +189,8 @@ main(int argc, char *argv[]) > if (gssd_check_mechs() != 0) > errx(1, "Problem with gssapi library"); > > - if (!fg && daemon(0, 0) < 0) > - errx(1, "fork"); > + if (!fg) > + mydaemon(0, 0, pipefds); > > signal(SIGINT, sig_die); > signal(SIGTERM, sig_die); > diff --git a/utils/gssd/gssd.h b/utils/gssd/gssd.h > index 86472a1..47995b7 100644 > --- a/utils/gssd/gssd.h > +++ b/utils/gssd/gssd.h > @@ -67,6 +67,7 @@ extern int use_memcache; > extern int root_uses_machine_creds; > extern unsigned int context_timeout; > extern char *preferred_realm; > +extern int pipefds[2]; > > TAILQ_HEAD(clnt_list_head, clnt_info) clnt_list; > > diff --git a/utils/gssd/gssd_main_loop.c b/utils/gssd/gssd_main_loop.c > index ccf7fe5..9970028 100644 > --- a/utils/gssd/gssd_main_loop.c > +++ b/utils/gssd/gssd_main_loop.c > @@ -53,6 +53,7 @@ > > #include "gssd.h" > #include "err_util.h" > +#include "nfslib.h" > > extern struct pollfd *pollarray; > extern unsigned long pollsize; > @@ -245,6 +246,9 @@ gssd_run() > /* Error msg is already printed */ > exit(1); > } > + > + /* release the parent after the initial dir scan */ > + release_parent(pipefds); > } > gssd_poll(pollarray, pollsize); > } > -- > 1.8.3.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Nov 18, 2013, at 11:38, J. Bruce Fields <bfields@fieldses.org> wrote: > On Fri, Nov 15, 2013 at 02:15:31PM -0500, Jeff Layton wrote: >> With some proposed kernel changes, it won't even attempt to upcall >> sometimes if it doesn't appear that gssd is running. This means that >> we have a theoretical race between gssd starting up at boot time and >> the init process attempting to mount kerberized filesystems. >> >> Fix this by switching gssd to use mydaemon() and having the child >> only release the parent after it has processed the directory once. > > Makes sense to me, thanks--ACK. We now appear to have 2 more or less identical copies of mydaemon in nfs-utils (one in utils/gssd/svcgssd.c and one in utils/idmapd/idmapd.c). Time to make it a common library function? -- Trond Myklebust Linux NFS client maintainer NetApp Trond.Myklebust@netapp.com www.netapp.com -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 18 Nov 2013 16:48:14 +0000 "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote: > > On Nov 18, 2013, at 11:38, J. Bruce Fields <bfields@fieldses.org> wrote: > > > On Fri, Nov 15, 2013 at 02:15:31PM -0500, Jeff Layton wrote: > >> With some proposed kernel changes, it won't even attempt to upcall > >> sometimes if it doesn't appear that gssd is running. This means that > >> we have a theoretical race between gssd starting up at boot time and > >> the init process attempting to mount kerberized filesystems. > >> > >> Fix this by switching gssd to use mydaemon() and having the child > >> only release the parent after it has processed the directory once. > > > > Makes sense to me, thanks--ACK. > > We now appear to have 2 more or less identical copies of mydaemon in nfs-utils (one in utils/gssd/svcgssd.c and one in utils/idmapd/idmapd.c). Time to make it a common library function? > See patch 1 in the series. That's exactly what I did...
On 15/11/13 14:15, Jeff Layton wrote: > With some proposed kernel changes, it won't even attempt to upcall > sometimes if it doesn't appear that gssd is running. This means that > we have a theoretical race between gssd starting up at boot time and > the init process attempting to mount kerberized filesystems. > > Fix this by switching gssd to use mydaemon() and having the child > only release the parent after it has processed the directory once. > > Signed-off-by: Jeff Layton <jlayton@redhat.com> Committed (tag: nfs-utils-1-2-10-rc1) steved. > --- > utils/gssd/gssd.c | 6 ++++-- > utils/gssd/gssd.h | 1 + > utils/gssd/gssd_main_loop.c | 4 ++++ > 3 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c > index 8ee478b..fdad153 100644 > --- a/utils/gssd/gssd.c > +++ b/utils/gssd/gssd.c > @@ -54,6 +54,7 @@ > #include "err_util.h" > #include "gss_util.h" > #include "krb5_util.h" > +#include "nfslib.h" > > char pipefs_dir[PATH_MAX] = GSSD_PIPEFS_DIR; > char keytabfile[PATH_MAX] = GSSD_DEFAULT_KEYTAB_FILE; > @@ -63,6 +64,7 @@ int use_memcache = 0; > int root_uses_machine_creds = 1; > unsigned int context_timeout = 0; > char *preferred_realm = NULL; > +int pipefds[2] = { -1, -1 }; > > void > sig_die(int signal) > @@ -187,8 +189,8 @@ main(int argc, char *argv[]) > if (gssd_check_mechs() != 0) > errx(1, "Problem with gssapi library"); > > - if (!fg && daemon(0, 0) < 0) > - errx(1, "fork"); > + if (!fg) > + mydaemon(0, 0, pipefds); > > signal(SIGINT, sig_die); > signal(SIGTERM, sig_die); > diff --git a/utils/gssd/gssd.h b/utils/gssd/gssd.h > index 86472a1..47995b7 100644 > --- a/utils/gssd/gssd.h > +++ b/utils/gssd/gssd.h > @@ -67,6 +67,7 @@ extern int use_memcache; > extern int root_uses_machine_creds; > extern unsigned int context_timeout; > extern char *preferred_realm; > +extern int pipefds[2]; > > TAILQ_HEAD(clnt_list_head, clnt_info) clnt_list; > > diff --git a/utils/gssd/gssd_main_loop.c b/utils/gssd/gssd_main_loop.c > index ccf7fe5..9970028 100644 > --- a/utils/gssd/gssd_main_loop.c > +++ b/utils/gssd/gssd_main_loop.c > @@ -53,6 +53,7 @@ > > #include "gssd.h" > #include "err_util.h" > +#include "nfslib.h" > > extern struct pollfd *pollarray; > extern unsigned long pollsize; > @@ -245,6 +246,9 @@ gssd_run() > /* Error msg is already printed */ > exit(1); > } > + > + /* release the parent after the initial dir scan */ > + release_parent(pipefds); > } > gssd_poll(pollarray, pollsize); > } > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c index 8ee478b..fdad153 100644 --- a/utils/gssd/gssd.c +++ b/utils/gssd/gssd.c @@ -54,6 +54,7 @@ #include "err_util.h" #include "gss_util.h" #include "krb5_util.h" +#include "nfslib.h" char pipefs_dir[PATH_MAX] = GSSD_PIPEFS_DIR; char keytabfile[PATH_MAX] = GSSD_DEFAULT_KEYTAB_FILE; @@ -63,6 +64,7 @@ int use_memcache = 0; int root_uses_machine_creds = 1; unsigned int context_timeout = 0; char *preferred_realm = NULL; +int pipefds[2] = { -1, -1 }; void sig_die(int signal) @@ -187,8 +189,8 @@ main(int argc, char *argv[]) if (gssd_check_mechs() != 0) errx(1, "Problem with gssapi library"); - if (!fg && daemon(0, 0) < 0) - errx(1, "fork"); + if (!fg) + mydaemon(0, 0, pipefds); signal(SIGINT, sig_die); signal(SIGTERM, sig_die); diff --git a/utils/gssd/gssd.h b/utils/gssd/gssd.h index 86472a1..47995b7 100644 --- a/utils/gssd/gssd.h +++ b/utils/gssd/gssd.h @@ -67,6 +67,7 @@ extern int use_memcache; extern int root_uses_machine_creds; extern unsigned int context_timeout; extern char *preferred_realm; +extern int pipefds[2]; TAILQ_HEAD(clnt_list_head, clnt_info) clnt_list; diff --git a/utils/gssd/gssd_main_loop.c b/utils/gssd/gssd_main_loop.c index ccf7fe5..9970028 100644 --- a/utils/gssd/gssd_main_loop.c +++ b/utils/gssd/gssd_main_loop.c @@ -53,6 +53,7 @@ #include "gssd.h" #include "err_util.h" +#include "nfslib.h" extern struct pollfd *pollarray; extern unsigned long pollsize; @@ -245,6 +246,9 @@ gssd_run() /* Error msg is already printed */ exit(1); } + + /* release the parent after the initial dir scan */ + release_parent(pipefds); } gssd_poll(pollarray, pollsize); }
With some proposed kernel changes, it won't even attempt to upcall sometimes if it doesn't appear that gssd is running. This means that we have a theoretical race between gssd starting up at boot time and the init process attempting to mount kerberized filesystems. Fix this by switching gssd to use mydaemon() and having the child only release the parent after it has processed the directory once. Signed-off-by: Jeff Layton <jlayton@redhat.com> --- utils/gssd/gssd.c | 6 ++++-- utils/gssd/gssd.h | 1 + utils/gssd/gssd_main_loop.c | 4 ++++ 3 files changed, 9 insertions(+), 2 deletions(-)