Skip to contentSkip to conversation

Current autofs patch queue

PATCH In review Ian Kent 4 participants 47 messages Aug 19, 2013 – Sep 6, 2013

Subsystems (3)
lore
Share & tools
lore

Ian Kent

author maintainer
#0 lore raw

cover Current autofs patch queue

There are patches from several contributors I plan on committing so I thought posting them to the list for final review would be sensible.

I'm not sure when I'll commit the changes but I hope in the next couple of days.

---

Dustin Polke (1):
autofs-5.0.7 - fix compilation of lookup_ldap.c without sasl

Ian Kent (13):
autofs-5.0.7 - don't override LDFLAGS in make rules autofs-5.0.7 - fix a couple of compiler warnings autofs-5.0.7 - add after sssd dependency to unit file autofs-5.0.7 - dont start readmap unless ready autofs-5.0.7 - fix dumpmaps multi output autofs-5.0.7 - try and cleanup after dumpmaps autofs-5.0.7 - teach dumpmaps to output simple key value pairs autofs-5.0.7 - fix syncronize handle_mounts() shutdown autofs-5.0.7 - fix fix wildcard multi map regression autofs-5.0.7 - improve timeout option description autofs-5.0.7 - only probe specific nfs version when requested autofs-5.0.7 - fix bad mkdir permission on create autofs-5.0.7 - setup program map env from macro table

Lan Yixun (dlan) (10):
autofs-5.0.7 - fix compile error with heimdal support enabled autofs-5.0.7 - fix typo forced-shutdown should be force-shutdown autofs-5.0.7 - fix hesiod check error and use correct $(LIBS) setting autofs-5.0.7 - fix dead LDAP symbolic link when LDAP support is disabled autofs-5.0.7 - add missing libtirpc lib to mount_nfs.so when TIRPC enabled autofs-5.0.7 - use compiler determined by configure instead of hard-coded ones autofs-5.0.7 - remove hard-coded STRIP variable
autofs-5.0.7 - use LIBS for link libraries
autofs-5.0.7 - unbundle NOTSTRIP from DEBUG so they dont depend on each other autofs-5.0.7 - fix occasional build error when enable parallel compiling

Leonardo Chiquitto (1):
autofs-5.0.7 - fix crash due to thread unsafe use of libldap

 CHANGELOG                      |   23 ++++
 Makefile.conf.in               |    3 
 Makefile.rules                 |   34 +++--
 aclocal.m4                     |   19 ++-
 autofs.spec                    |    3 
 configure                      |  104 +++++++++++++++--
 configure.in                   |    2 
 daemon/Makefile                |    2 
 daemon/automount.c             |   71 +++++++++--
 daemon/state.c                 |   15 +-
 include/config.h.in            |    3 
 include/lookup_ldap.h          |    4 -
 include/macros.h               |    1 
 include/master.h               |    1 
 lib/Makefile                   |    6 +
 lib/macros.c                   |   28 ++++
 lib/master.c                   |  250 ++++++++++++++++++++++++++++++++++++++--
 man/auto.master.5.in           |    5 +
 man/automount.8                |   23 +++-
 modules/Makefile               |   20 ++-
 modules/cyrus-sasl.c           |   39 +++++-
 modules/lookup_file.c          |   25 +++-
 modules/lookup_ldap.c          |   76 +++++++++++-
 modules/lookup_nisplus.c       |   26 +++-
 modules/lookup_program.c       |   20 +++
 modules/lookup_sss.c           |   22 +++-
 modules/lookup_yp.c            |   23 +++-
 modules/mount_bind.c           |    2 
 modules/mount_nfs.c            |   14 ++
 redhat/autofs.sysconfig.in     |    5 +
 samples/autofs.conf.default.in |    5 +
 samples/autofs.service.in      |    2 
 32 files changed, 753 insertions(+), 123 deletions(-)
Signature · 1 line
Ian
author maintainer
#1 lore raw

PATCH 1/25 autofs-5.0.7 - don't override LDFLAGS in make rules

From: Ian Kent <[email protected]>

Ensure that externally defined LDFLAGS is not overridden and ensure binary is linked as RELRO in tar spec file.

---
 CHANGELOG      |    1 +
 Makefile.rules |    6 +++---
 autofs.spec    |    3 ++-
 3 files changed, 6 insertions(+), 4 deletions(-)
CHANGELOG +1 -0
index 5d90139..37eac72 100644
@@ -53,6 +53,7 @@
 - fix probe each nfs version in turn for singleton mounts.
 - misc man page fixes.
 - fix add null check in parse_server_string().
+- don't override LDFLAGS in make rules.
 
 25/07/2012 autofs-5.0.7
 =======================
Makefile.rules +3 -3
index f2ba386..6b5b2bd 100644
@@ -24,16 +24,16 @@ endif
 
 ifdef DEBUG
 CFLAGS    ?= -g -Wall -DDEBUG
-LDFLAGS   = -g
+LDFLAGS   ?= -g
 STRIP     = :
 else
 ifdef DONTSTRIP
 CFLAGS    ?= -O2 -g
-LDFLAGS   = -g
+LDFLAGS   ?= -g
 STRIP     = :
 else
 CFLAGS    ?= -O2 -Wall
-LDFLAGS   = -s
+LDFLAGS   ?= -s
 STRIP     = strip --strip-debug
 endif
 endif
autofs.spec +2 -1
index a768e44..9a2bf6e 100644
@@ -85,12 +85,13 @@ echo %{version}-%{release} > .version
 
 %build
 CFLAGS="$RPM_OPT_FLAGS -Wall" \
+LDFLAGS="-Wl,-z,now" \
 ./configure --libdir=%{_libdir} \
 	--disable-mount-locking \
 	--enable-ignore-busy \
 	%{?systemd_configure_arg:} \
 	%{?libtirpc_configure_arg:}
-CFLAGS="$RPM_OPT_FLAGS -Wall" make initdir=/etc/rc.d/init.d DONTSTRIP=1
+CFLAGS="$RPM_OPT_FLAGS -Wall" LDFLAGS="-Wl,-z,now" make initdir=/etc/rc.d/init.d DONTSTRIP=1
 
 %install
 rm -rf $RPM_BUILD_ROOT
author maintainer
#2 lore raw

PATCH 2/25 autofs-5.0.7 - fix a couple of compiler warnings

In reply to Ian Kent

From: Ian Kent <[email protected]>

Quiet a couple of ompiler warnings by making intention explicit using brackets.

---
 CHANGELOG            |    1 +
 modules/mount_bind.c |    2 +-
 modules/mount_nfs.c  |    4 ++--
 3 files changed, 4 insertions(+), 3 deletions(-)
CHANGELOG +1 -0
index 37eac72..7749f01 100644
@@ -54,6 +54,7 @@
 - misc man page fixes.
 - fix add null check in parse_server_string().
 - don't override LDFLAGS in make rules.
+- fix a couple of compiler warnings.
 
 25/07/2012 autofs-5.0.7
 =======================
modules/mount_bind.c +1 -1
index 61a773c..2b70104 100644
@@ -208,7 +208,7 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int
 			      MODPREFIX
 			      "failed to create local mount %s -> %s",
 			      fullpath, what);
-			if (ap->flags & MOUNT_FLAG_GHOST && !status)
+			if ((ap->flags & MOUNT_FLAG_GHOST) && !status)
 				if (mkdir_path(fullpath, 0555) && errno != EEXIST) {
 					char *estr = strerror_r(errno, buf, MAX_ERR_BUF);
 					error(ap->logopt,
modules/mount_nfs.c +2 -2
index 3d2ccea..adf2002 100644
@@ -202,8 +202,8 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int
 	 */
 	if ((hosts && !hosts->next) &&
 	    mount_default_proto == 4 &&
-	    vers & NFS_VERS_MASK != 0 &&
-	    vers & NFS4_VERS_MASK != 0) {
+	    (vers & NFS_VERS_MASK) != 0 &&
+	    (vers & NFS4_VERS_MASK) != 0) {
 		unsigned int v4_probe_ok = 0;
 		struct host *tmp = new_host(hosts->name,
 					    hosts->addr, hosts->addr_len,
author maintainer
#3 lore raw

PATCH 3/25 autofs-5.0.7 - add after sssd dependency to unit file

In reply to Ian Kent

From: Ian Kent <[email protected]>

When using sss as a map source autofs can sometimes start before sssd is ready.

---
 CHANGELOG                 |    1 +
 samples/autofs.service.in |    2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)
CHANGELOG +1 -0
index 7749f01..2734fe3 100644
@@ -55,6 +55,7 @@
 - fix add null check in parse_server_string().
 - don't override LDFLAGS in make rules.
 - fix a couple of compiler warnings.
+- add after sssd dependency to unit file.
 
 25/07/2012 autofs-5.0.7
 =======================
samples/autofs.service.in +1 -1
index 056ab5e..777463d 100644
@@ -1,6 +1,6 @@
 [Unit]
 Description=Automounts filesystems on demand
-After=network.target ypbind.service
+After=network.target ypbind.service sssd.service
 
 [Service]
 Type=forking
author maintainer
#4 lore raw

PATCH 4/25 autofs-5.0.7 - dont start readmap unless ready

In reply to Ian Kent

From: Ian Kent <[email protected]>

State transitions should only be made when an autofs point goes to state ST_READY or when a new task is added to an empty task queue.

---
 CHANGELOG      |    1 +
 daemon/state.c |   15 +++++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)
CHANGELOG +1 -0
index 2734fe3..3228d6b 100644
@@ -56,6 +56,7 @@
 - don't override LDFLAGS in make rules.
 - fix a couple of compiler warnings.
 - add after sssd dependency to unit file.
+- dont start readmap unless ready.
 
 25/07/2012 autofs-5.0.7
 =======================
daemon/state.c +9 -6
index ddc5556..8d81788 100644
@@ -818,14 +818,13 @@ done:
 		new = st_alloc_task(ap, state);
 		if (new)
 			list_add(&new->list, head);
+		/* Added to empty state queue, kick state machine */
+		signaled = 1;
+		status = pthread_cond_signal(&cond);
+		if (status)
+			fatal(status);
 	}
 
-	/* Added task, encourage state machine */
-	signaled = 1;
-	status = pthread_cond_signal(&cond);
-	if (status)
-		fatal(status);
-
 	return 1;
 }
 
@@ -1131,6 +1130,10 @@ static void *st_queue_handler(void *arg)
 				task = list_entry(p, struct state_queue, list);
 				p = p->next;
 
+				/* Task may have been canceled before it started */
+				if (!task->thid && task->cancel)
+					goto remove;
+
 				if (!task->busy) {
 					/* Start a new task */
 					task->busy = 1;
author maintainer
#5 lore raw

PATCH 5/25 autofs-5.0.7 - fix crash due to thread unsafe use of libldap

In reply to Ian Kent

From: Leonardo Chiquitto <[email protected]>

Add locking around LDAP initialization calls

To prevent corruption inside SSL and LDAP libraries, it's necessary to serialize all calls to functions that initialize LDAP contexts.

How to reproduce the problem:

- Setup an LDAP server with SSL/TLS support enabled
- Configure AutoFS to fetch the maps from LDAP
- Make sure the OpenLDAP client library is configured to use SSL connections and "usetls" is set to yes in autofs_ldap_auth.conf.

In one directory handled by AutoFS (an indirect mount point), trigger in parallel some dozens of invalid mounts (ie, try to access keys that do not exist in the AutoFS map). Repeat until it crashes.

Here it always crashes in less than 20 minutes, normally inside OpenSSL. Core dump inspection shows that internal SSL structures are corrupted, with function pointers pointing to random addresses.

Trying to find similar reports on the web, I found this email from an OpenLDAP developer (partial quote, emphasis mine) [1]:

"As far as I know, libldap is thread safe in the sense that multiple threads can use separate LDAP* handles without running into concurrency issues; *except for library initialization*, all accesses to common data (i.e. global variables) is read-only."

[1]http://www.openldap.org/cgi-bin/wilma_hiliter/openldap-software/200606/msg00252.html

AutoFS implements no locking around LDAP initialization libraries and it's quite common to see multiple threads executing ldap_initialize() or ldap_start_tls_s() at the same time.

---
 CHANGELOG             |    1 +
 modules/lookup_ldap.c |   35 ++++++++++++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)
CHANGELOG +1 -0
index 3228d6b..fe232f4 100644
@@ -57,6 +57,7 @@
 - fix a couple of compiler warnings.
 - add after sssd dependency to unit file.
 - dont start readmap unless ready.
+- fix crash due to thread unsafe use of libldap.
 
 25/07/2012 autofs-5.0.7
 =======================
modules/lookup_ldap.c +34 -1
index a2bfafd..655e9fa 100644
@@ -52,6 +52,12 @@ static struct ldap_schema common_schema[] = {
 };
 static unsigned int common_schema_count = sizeof(common_schema)/sizeof(struct ldap_schema);
 
+/*
+ * Initialization of LDAP and OpenSSL must be always serialized to
+ * avoid corruption of context structures inside these libraries.
+ */
+pthread_mutex_t ldapinit_mutex = PTHREAD_MUTEX_INITIALIZER;
+
 struct ldap_search_params {
 	struct autofs_point *ap;
 	LDAP *ldap;
@@ -136,6 +142,22 @@ int ldap_parse_page_control(LDAP *ldap, LDAPControl **controls,
 }
 #endif /* HAVE_LDAP_PARSE_PAGE_CONTROL */
 
+static void ldapinit_mutex_lock(void)
+{
+	int status = pthread_mutex_lock(&ldapinit_mutex);
+	if (status)
+		fatal(status);
+	return;
+}
+
+static void ldapinit_mutex_unlock(void)
+{
+	int status = pthread_mutex_unlock(&ldapinit_mutex);
+	if (status)
+		fatal(status);
+	return;
+}
+
 static void uris_mutex_lock(struct lookup_context *ctxt)
 {
 	int status = pthread_mutex_lock(&ctxt->uris_mutex);
@@ -196,7 +218,7 @@ int unbind_ldap_connection(unsigned logopt, LDAP *ldap, struct lookup_context *c
 	return rv;
 }
 
-LDAP *init_ldap_connection(unsigned logopt, const char *uri, struct lookup_context *ctxt)
+LDAP *__init_ldap_connection(unsigned logopt, const char *uri, struct lookup_context *ctxt)
 {
 	LDAP *ldap = NULL;
 	struct timeval timeout     = { ctxt->timeout, 0 };
@@ -277,6 +299,17 @@ LDAP *init_ldap_connection(unsigned logopt, const char *uri, struct lookup_conte
 	return ldap;
 }
 
+LDAP *init_ldap_connection(unsigned logopt, const char *uri, struct lookup_context *ctxt)
+{
+	LDAP *ldap;
+
+	ldapinit_mutex_lock();
+	ldap = __init_ldap_connection(logopt, uri, ctxt);
+	ldapinit_mutex_unlock();
+
+	return ldap;
+}
+
 static int get_query_dn(unsigned logopt, LDAP *ldap, struct lookup_context *ctxt, const char *class, const char *key)
 {
 	char buf[MAX_ERR_BUF];
author maintainer
#6 lore raw

PATCH 6/25 autofs-5.0.7 - fix compile error with heimdal support enabled

In reply to Ian Kent

From: Lan Yixun (dlan) <[email protected]>

this patch instroduce a compatible layer between Heimdal and MTT Krb5. And I slightly rework the original patch to make it more readable.

Upstream Discussion:
http://thread.gmane.org/gmane.linux.kernel.autofs/4203

Gentoo Bugs:
https://bugs.gentoo.org/show_bug.cgi?id=210762

Edited by: Ian Kent <[email protected]>
- fix code indentation.
- update configure to include change.

---
 CHANGELOG            |    1 +
 aclocal.m4           |    7 ++++
 configure            |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/config.h.in  |    3 ++
 modules/cyrus-sasl.c |   39 ++++++++++++++++++++---
 5 files changed, 129 insertions(+), 5 deletions(-)
CHANGELOG +1 -0
index fe232f4..c77be18 100644
@@ -58,6 +58,7 @@
 - add after sssd dependency to unit file.
 - dont start readmap unless ready.
 - fix crash due to thread unsafe use of libldap.
+- fix compile error with heimdal support enabled.
 
 25/07/2012 autofs-5.0.7
 =======================
aclocal.m4 +7 -0
index c5de159..7a8b03c 100644
@@ -299,6 +299,13 @@ else
   HAVE_KRB5=1
   KRB5_LIBS=`$KRB5_CONFIG --libs`
   KRB5_FLAGS=`$KRB5_CONFIG --cflags`
+
+  SAVE_CFLAGS=$CFLAGS
+  SAVE_LIBS=$LIBS
+  CFLAGS="$CFLAGS $KRB5_FLAGS"
+  LIBS="$LIBS $KRB5_LIBS"
+
+  AC_CHECK_FUNCS([krb5_principal_get_realm])
 fi])
 
 dnl --------------------------------------------------------------------------
configure +84 -0

96 lines not on this page · raw message

include/config.h.in +3 -0
index a2a05a8..8a52080 100644
@@ -24,6 +24,9 @@
 /* Define to 1 if you have the <inttypes.h> header file. */
 #undef HAVE_INTTYPES_H
 
+/* Define to 1 if you have the `krb5_principal_get_realm' function. */
+#undef HAVE_KRB5_PRINCIPAL_GET_REALM
+
 /* Define to 1 if you have the `ldap_create_page_control' function. */
 #undef HAVE_LDAP_CREATE_PAGE_CONTROL
 
modules/cyrus-sasl.c +34 -5
index 68f9242..b9c16f3 100644
@@ -64,6 +64,35 @@
 #endif
 #endif
 
+#ifdef HAVE_KRB5_PRINCIPAL_GET_REALM
+void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
+                          const char **realm, int *len)
+{
+	*realm = krb5_principal_get_realm(context, princ);
+	if (*realm)
+		*len = strlen(*realm);
+	else
+		*len = 0;
+	return;
+#else
+void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
+                          const char **realm, int *len)
+{
+	const krb5_data *data;
+
+	data = krb5_princ_realm(context, princ);
+	if (data) {
+		*realm = data->data;
+		*len = data->length;
+	} else {
+		*realm = NULL;
+		*len = 0;
+	}
+	return;
+}
+#endif
+
+
 /*
  *  Once a krb5 credentials cache is setup, we need to set the KRB5CCNAME
  *  environment variable so that the library knows where to find it.
@@ -379,7 +408,8 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt)
 	krb5_principal tgs_princ, krb5_client_princ;
 	krb5_creds my_creds;
 	char *tgs_name;
-	int status;
+	const char *realm_name;
+	int status, realm_length;
 
 	if (ctxt->kinit_done)
 		return 0;
@@ -450,12 +480,11 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt)
 	}
 
 	/* setup a principal for the ticket granting service */
+	_krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ, &realm_name, &realm_length);
 	ret = krb5_build_principal_ext(ctxt->krb5ctxt, &tgs_princ,
-		krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->length,
-		krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->data,
+		realm_length, realm_name,
 		strlen(KRB5_TGS_NAME), KRB5_TGS_NAME,
-		krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->length,
-		krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->data,
+		realm_length, realm_name,
 		0);
 	if (ret) {
 		error(logopt,
#7 lore raw

Re: [PATCH 06/25] autofs-5.0.7 - fix compile error with heimdal support enabled

Quoted 178 lines · Ian Kent

From: Lan Yixun (dlan) <[email protected]>

this patch instroduce a compatible layer between Heimdal and MTT Krb5. And I slightly rework the original patch to make it more readable.

Upstream Discussion:
http://thread.gmane.org/gmane.linux.kernel.autofs/4203

Gentoo Bugs:
https://bugs.gentoo.org/show_bug.cgi?id=210762

Edited by: Ian Kent <[email protected]>
- fix code indentation.
- update configure to include change.

---
 CHANGELOG            |    1 +
 aclocal.m4           |    7 ++++
 configure            |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/config.h.in  |    3 ++
 modules/cyrus-sasl.c |   39 ++++++++++++++++++++---
 5 files changed, 129 insertions(+), 5 deletions(-)
CHANGELOG +1 -0
index fe232f4..c77be18 100644
@@ -58,6 +58,7 @@
 - add after sssd dependency to unit file.
 - dont start readmap unless ready.
 - fix crash due to thread unsafe use of libldap.
+- fix compile error with heimdal support enabled.
 
 25/07/2012 autofs-5.0.7
 =======================
aclocal.m4 +7 -0
index c5de159..7a8b03c 100644
@@ -299,6 +299,13 @@ else
   HAVE_KRB5=1
   KRB5_LIBS=`$KRB5_CONFIG --libs`
   KRB5_FLAGS=`$KRB5_CONFIG --cflags`
+
+  SAVE_CFLAGS=$CFLAGS
+  SAVE_LIBS=$LIBS
+  CFLAGS="$CFLAGS $KRB5_FLAGS"
+  LIBS="$LIBS $KRB5_LIBS"
+
+  AC_CHECK_FUNCS([krb5_principal_get_realm])
 fi])
 
 dnl --------------------------------------------------------------------------
configure +84 -0
index 08b7ccc..1086eba 100755
@@ -1559,6 +1559,73 @@ fi
 
 } # ac_fn_c_try_link
 
+# ac_fn_c_check_func LINENO FUNC VAR
+# ----------------------------------
+# Tests whether FUNC exists, setting the cache variable VAR accordingly
+ac_fn_c_check_func ()
+{
+  as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $2" >&5
+$as_echo_n "checking for $2... " >&6; }
+if eval \${$3+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+/* Define $2 to an innocuous variant, in case <limits.h> declares $2.
+   For example, HP-UX 11i <limits.h> declares gettimeofday.  */
+#define $2 innocuous_$2
+
+/* System header to define __stub macros and hopefully few prototypes,
+    which can conflict with char $2 (); below.
+    Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
+    <limits.h> exists even on freestanding compilers.  */
+
+#ifdef __STDC__
+# include <limits.h>
+#else
+# include <assert.h>
+#endif
+
+#undef $2
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char $2 ();
+/* The GNU C library defines this for functions which it implements
+    to always fail with ENOSYS.  Some functions are actually named
+    something starting with __ and the normal name is an alias.  */
+#if defined __stub_$2 || defined __stub___$2
+choke me
+#endif
+
+int
+main ()
+{
+return $2 ();
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  eval "$3=yes"
+else
+  eval "$3=no"
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+fi
+eval ac_res=\$$3
+              { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
+$as_echo "$ac_res" >&6; }
+  eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
+
+} # ac_fn_c_check_func
+
 # ac_fn_c_try_cpp LINENO
 # ----------------------
 # Try to preprocess conftest.$ac_ext, and return whether this succeeded.
@@ -4039,6 +4106,23 @@ $as_echo "yes" >&6; }
   HAVE_KRB5=1
   KRB5_LIBS=`$KRB5_CONFIG --libs`
   KRB5_FLAGS=`$KRB5_CONFIG --cflags`
+
+  SAVE_CFLAGS=$CFLAGS
+  SAVE_LIBS=$LIBS
+  CFLAGS="$CFLAGS $KRB5_FLAGS"
+  LIBS="$LIBS $KRB5_LIBS"
+
+  for ac_func in krb5_principal_get_realm
+do :
+  ac_fn_c_check_func "$LINENO" "krb5_principal_get_realm" "ac_cv_func_krb5_principal_get_realm"
+if test "x$ac_cv_func_krb5_principal_get_realm" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_KRB5_PRINCIPAL_GET_REALM 1
+_ACEOF
+
+fi
+done
+
 fi
 
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing versionsort" >&5
include/config.h.in +3 -0
index a2a05a8..8a52080 100644
@@ -24,6 +24,9 @@
 /* Define to 1 if you have the <inttypes.h> header file. */
 #undef HAVE_INTTYPES_H
 
+/* Define to 1 if you have the `krb5_principal_get_realm' function. */
+#undef HAVE_KRB5_PRINCIPAL_GET_REALM
+
 /* Define to 1 if you have the `ldap_create_page_control' function. */
 #undef HAVE_LDAP_CREATE_PAGE_CONTROL
 
modules/cyrus-sasl.c +0 -0
index 68f9242..b9c16f3 100644
@@ -64,6 +64,35 @@
 #endif
 #endif
 

modules/cyrus-sasl.c line 76

Quoted 10 lines
modules/cyrus-sasl.c +10 -0
+#ifdef HAVE_KRB5_PRINCIPAL_GET_REALM
+void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
+                          const char **realm, int *len)
+{
+       *realm = krb5_principal_get_realm(context, princ);
+       if (*realm)
+               *len = strlen(*realm);
+       else
+               *len = 0;
+       return;

missed '}' here

Quoted 49 lines
modules/cyrus-sasl.c +24 -5
+#else
+void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
+                          const char **realm, int *len)
+{
+       const krb5_data *data;
+
+       data = krb5_princ_realm(context, princ);
+       if (data) {
+               *realm = data->data;
+               *len = data->length;
+       } else {
+               *realm = NULL;
+               *len = 0;
+       }
+       return;
+}
+#endif
+
+
 /*
  *  Once a krb5 credentials cache is setup, we need to set the KRB5CCNAME
  *  environment variable so that the library knows where to find it.
@@ -379,7 +408,8 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt)
        krb5_principal tgs_princ, krb5_client_princ;
        krb5_creds my_creds;
        char *tgs_name;
-       int status;
+       const char *realm_name;
+       int status, realm_length;
 
        if (ctxt->kinit_done)
                return 0;
@@ -450,12 +480,11 @@ sasl_do_kinit(unsigned logopt, struct lookup_context *ctxt)
        }
 
        /* setup a principal for the ticket granting service */
+       _krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ, &realm_name, &realm_length);
        ret = krb5_build_principal_ext(ctxt->krb5ctxt, &tgs_princ,
-               krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->length,
-               krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->data,
+               realm_length, realm_name,
                strlen(KRB5_TGS_NAME), KRB5_TGS_NAME,
-               krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->length,
-               krb5_princ_realm(ctxt->krb5ctxt, krb5_client_princ)->data,
+               realm_length, realm_name,
                0);
        if (ret) {
                error(logopt,
author maintainer
#8 lore raw

Re: [PATCH 06/25] autofs-5.0.7 - fix compile error with heimdal support enabled

Quoted 179 lines · Dennis Lan (dlan)
Quoted 178 lines · Ian Kent

From: Lan Yixun (dlan) <[email protected]>

this patch instroduce a compatible layer between Heimdal and MTT Krb5. And I slightly rework the original patch to make it more readable.

Upstream Discussion:
http://thread.gmane.org/gmane.linux.kernel.autofs/4203

Gentoo Bugs:
https://bugs.gentoo.org/show_bug.cgi?id=210762

Edited by: Ian Kent <[email protected]>
- fix code indentation.
- update configure to include change.

---
 CHANGELOG            |    1 +
 aclocal.m4           |    7 ++++
 configure            |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/config.h.in  |    3 ++
 modules/cyrus-sasl.c |   39 ++++++++++++++++++++---
 5 files changed, 129 insertions(+), 5 deletions(-)
CHANGELOG +1 -0
index fe232f4..c77be18 100644
@@ -58,6 +58,7 @@
 - add after sssd dependency to unit file.
 - dont start readmap unless ready.
 - fix crash due to thread unsafe use of libldap.
+- fix compile error with heimdal support enabled.
 
 25/07/2012 autofs-5.0.7
 =======================
aclocal.m4 +7 -0
index c5de159..7a8b03c 100644
@@ -299,6 +299,13 @@ else
   HAVE_KRB5=1
   KRB5_LIBS=`$KRB5_CONFIG --libs`
   KRB5_FLAGS=`$KRB5_CONFIG --cflags`
+
+  SAVE_CFLAGS=$CFLAGS
+  SAVE_LIBS=$LIBS
+  CFLAGS="$CFLAGS $KRB5_FLAGS"
+  LIBS="$LIBS $KRB5_LIBS"
+
+  AC_CHECK_FUNCS([krb5_principal_get_realm])
 fi])
 
 dnl --------------------------------------------------------------------------
configure +84 -0
index 08b7ccc..1086eba 100755
@@ -1559,6 +1559,73 @@ fi
 
 } # ac_fn_c_try_link
 
+# ac_fn_c_check_func LINENO FUNC VAR
+# ----------------------------------
+# Tests whether FUNC exists, setting the cache variable VAR accordingly
+ac_fn_c_check_func ()
+{
+  as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $2" >&5
+$as_echo_n "checking for $2... " >&6; }
+if eval \${$3+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+/* Define $2 to an innocuous variant, in case <limits.h> declares $2.
+   For example, HP-UX 11i <limits.h> declares gettimeofday.  */
+#define $2 innocuous_$2
+
+/* System header to define __stub macros and hopefully few prototypes,
+    which can conflict with char $2 (); below.
+    Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
+    <limits.h> exists even on freestanding compilers.  */
+
+#ifdef __STDC__
+# include <limits.h>
+#else
+# include <assert.h>
+#endif
+
+#undef $2
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char $2 ();
+/* The GNU C library defines this for functions which it implements
+    to always fail with ENOSYS.  Some functions are actually named
+    something starting with __ and the normal name is an alias.  */
+#if defined __stub_$2 || defined __stub___$2
+choke me
+#endif
+
+int
+main ()
+{
+return $2 ();
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  eval "$3=yes"
+else
+  eval "$3=no"
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+fi
+eval ac_res=\$$3
+              { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
+$as_echo "$ac_res" >&6; }
+  eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
+
+} # ac_fn_c_check_func
+
 # ac_fn_c_try_cpp LINENO
 # ----------------------
 # Try to preprocess conftest.$ac_ext, and return whether this succeeded.
@@ -4039,6 +4106,23 @@ $as_echo "yes" >&6; }
   HAVE_KRB5=1
   KRB5_LIBS=`$KRB5_CONFIG --libs`
   KRB5_FLAGS=`$KRB5_CONFIG --cflags`
+
+  SAVE_CFLAGS=$CFLAGS
+  SAVE_LIBS=$LIBS
+  CFLAGS="$CFLAGS $KRB5_FLAGS"
+  LIBS="$LIBS $KRB5_LIBS"
+
+  for ac_func in krb5_principal_get_realm
+do :
+  ac_fn_c_check_func "$LINENO" "krb5_principal_get_realm" "ac_cv_func_krb5_principal_get_realm"
+if test "x$ac_cv_func_krb5_principal_get_realm" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_KRB5_PRINCIPAL_GET_REALM 1
+_ACEOF
+
+fi
+done
+
 fi
 
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing versionsort" >&5
include/config.h.in +3 -0
index a2a05a8..8a52080 100644
@@ -24,6 +24,9 @@
 /* Define to 1 if you have the <inttypes.h> header file. */
 #undef HAVE_INTTYPES_H
 
+/* Define to 1 if you have the `krb5_principal_get_realm' function. */
+#undef HAVE_KRB5_PRINCIPAL_GET_REALM
+
 /* Define to 1 if you have the `ldap_create_page_control' function. */
 #undef HAVE_LDAP_CREATE_PAGE_CONTROL
 
modules/cyrus-sasl.c +0 -0
index 68f9242..b9c16f3 100644
@@ -64,6 +64,35 @@
 #endif
 #endif
 
Quoted 12 lines

modules/cyrus-sasl.c line 76

Quoted 10 lines
modules/cyrus-sasl.c +10 -0
+#ifdef HAVE_KRB5_PRINCIPAL_GET_REALM
+void _krb5_princ_realm(krb5_context context, krb5_const_principal princ,
+                          const char **realm, int *len)
+{
+       *realm = krb5_principal_get_realm(context, princ);
+       if (*realm)
+               *len = strlen(*realm);
+       else
+               *len = 0;
+       return;

missed '}' here

Fixed and pushed to temporary pending branch, patch also updated in master branch.

Ian

author maintainer
#9 lore raw

PATCH 7/25 autofs-5.0.7 - fix typo forced-shutdown should be force-shutdown

In reply to Ian Kent

From: Lan Yixun (dlan) <[email protected]>

This will confuse people when they find what shows with "./configure --help" is different from what is actually used.

Edited by: Ian Kent <[email protected]>
- make description a little more readable.
- update configure to include change.

---
 CHANGELOG    |    1 +
 configure    |    8 ++++----
 configure.in |    2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)
CHANGELOG +1 -0
index c77be18..5265f6c 100644
@@ -59,6 +59,7 @@
 - dont start readmap unless ready.
 - fix crash due to thread unsafe use of libldap.
 - fix compile error with heimdal support enabled.
+- fix typo forced-shutdown should be force-shutdown.
 
 25/07/2012 autofs-5.0.7
 =======================
configure +4 -4
index 1086eba..b8e74b7 100755
@@ -741,7 +741,7 @@ with_openldap
 with_sasl
 enable_ext_env
 enable_mount_locking
-enable_forced_shutdown
+enable_force_shutdown
 enable_ignore_busy
 '
       ac_precious_vars='build_alias
@@ -5637,9 +5637,9 @@ fi
 #
 # Enable forced shutdown on USR1 signal (unlink umounts all mounts).
 #
-# Check whether --enable-forced-shutdown was given.
-if test "${enable_forced_shutdown+set}" = set; then :
-  enableval=$enable_forced_shutdown;
+# Check whether --enable-force-shutdown was given.
+if test "${enable_force_shutdown+set}" = set; then :
+  enableval=$enable_force_shutdown;
 else
   enableval=no
 fi
configure.in +1 -1
index 559045a..0013a39 100644
@@ -363,7 +363,7 @@ fi
 #
 # Enable forced shutdown on USR1 signal (unlink umounts all mounts).
 #
-AC_ARG_ENABLE(forced-shutdown,
+AC_ARG_ENABLE(force-shutdown,
 [  --enable-force-shutdown enable USR1 signal to force unlink umount of any
 			  busy mounts during shutdown],,
 	enableval=no)
author maintainer
#10 lore raw

PATCH 8/25 autofs-5.0.7 - fix hesiod check error and use correct $(LIBS) setting

In reply to Ian Kent

From: Lan Yixun (dlan) <[email protected]>

the problem here is that hesiod checking always fail due to wrong $(LDFLAGS), no matter hesiod library is installed or not. moving "-lhesiod" into $(LIBS) solve this problem.

1) convert "char *" -> "void *" to fix the compile warning
2) put "-lhesiod -lresolv" into $(LIBS), not into $(LDFLAGS), otherwise we got link error

Edited by: Ian Kent <[email protected]>
- update configure to include change.

---
 CHANGELOG  |    1 +
 aclocal.m4 |   12 ++++++------
 configure  |   12 ++++++------
 3 files changed, 13 insertions(+), 12 deletions(-)
CHANGELOG +1 -0
index 5265f6c..647504b 100644
@@ -60,6 +60,7 @@
 - fix crash due to thread unsafe use of libldap.
 - fix compile error with heimdal support enabled.
 - fix typo forced-shutdown should be force-shutdown.
+- fix hesiod check error and use correct $(LIBS) setting.
 
 25/07/2012 autofs-5.0.7
 =======================
aclocal.m4 +6 -6
index 7a8b03c..637a775 100644
@@ -316,20 +316,20 @@ dnl --------------------------------------------------------------------------
 AC_DEFUN([AF_CHECK_LIBHESIOD],
 [AC_MSG_CHECKING(for libhesiod)
 
-# save current ldflags
-af_check_hesiod_save_ldflags="$LDFLAGS"
-LDFLAGS="$LDFLAGS -lhesiod -lresolv"
+# save current libs
+af_check_hesiod_save_libs="$LIBS"
+LIBS="$LIBS -lhesiod -lresolv"
 
 AC_TRY_LINK(
   [ #include <hesiod.h> ],
-  [ char *c; hesiod_init(&c); ],
+  [ void *c; hesiod_init(&c); ],
   [ HAVE_HESIOD=1
     LIBHESIOD="$LIBHESIOD -lhesiod -lresolv"
     AC_MSG_RESULT(yes) ],
   [ AC_MSG_RESULT(no) ])
 
-# restore ldflags
-LDFLAGS="$af_check_hesiod_save_ldflags"
+# restore libs
+LIBS="$af_check_hesiod_save_libs"
 ])
 
 dnl --------------------------------------------------------------------------
configure +6 -6
index b8e74b7..e6d5d4a 100755
@@ -4305,9 +4305,9 @@ then
 	{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for libhesiod" >&5
 $as_echo_n "checking for libhesiod... " >&6; }
 
-# save current ldflags
-af_check_hesiod_save_ldflags="$LDFLAGS"
-LDFLAGS="$LDFLAGS -lhesiod -lresolv"
+# save current libs
+af_check_hesiod_save_libs="$LIBS"
+LIBS="$LIBS -lhesiod -lresolv"
 
 cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
@@ -4315,7 +4315,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 int
 main ()
 {
- char *c; hesiod_init(&c);
+ void *c; hesiod_init(&c);
   ;
   return 0;
 }
@@ -4332,8 +4332,8 @@ fi
 rm -f core conftest.err conftest.$ac_objext \
     conftest$ac_exeext conftest.$ac_ext
 
-# restore ldflags
-LDFLAGS="$af_check_hesiod_save_ldflags"
+# restore libs
+LIBS="$af_check_hesiod_save_libs"
 
 	if test "$HAVE_HESIOD" == "1"; then
 
author maintainer
#11 lore raw

PATCH 9/25 autofs-5.0.7 - fix dead LDAP symbolic link when LDAP support is disabled

In reply to Ian Kent

From: Lan Yixun (dlan) <[email protected]>

autofs will create symbol link mandatory no matter ldap support is enabled or not. so, without this patch, lookup_ldaps.so will become a dead link.

Edited by: Ian Kent <[email protected]>
- change check from SASL to LDAP since the ldaps lookup module may still be used by ldaps:// as long as LDAP support is built.

---
 CHANGELOG        |    1 +
 modules/Makefile |    2 ++
 2 files changed, 3 insertions(+)
CHANGELOG +1 -0
index 647504b..296aaf8 100644
@@ -61,6 +61,7 @@
 - fix compile error with heimdal support enabled.
 - fix typo forced-shutdown should be force-shutdown.
 - fix hesiod check error and use correct $(LIBS) setting.
+- fix dead LDAP symbolic link when LDAP support is disabled.
 
 25/07/2012 autofs-5.0.7
 =======================
modules/Makefile +2 -0
index c5deb24..8c0df18 100644
@@ -74,7 +74,9 @@ install: all
 	-rm -f $(INSTALLROOT)$(autofslibdir)/mount_smbfs.so
 	ln -fs lookup_file.so $(INSTALLROOT)$(autofslibdir)/lookup_files.so
 	ln -fs lookup_yp.so $(INSTALLROOT)$(autofslibdir)/lookup_nis.so
+ifeq ($(LDAP), 1)
 	ln -fs lookup_ldap.so $(INSTALLROOT)$(autofslibdir)/lookup_ldaps.so
+endif
 	ln -fs mount_nfs.so $(INSTALLROOT)$(autofslibdir)/mount_nfs4.so
 ifeq ($(EXT2FS), 1)
  ifeq ($(EXT3FS), 1)
author maintainer
#12 lore raw

PATCH 10/25 autofs-5.0.7 - add missing libtirpc lib to mount_nfs.so when TIRPC enabled

In reply to Ian Kent

From: Lan Yixun (dlan) <[email protected]>

this is need when libtirpc is enabled, otherwise autofs fail to start due to it can't find symbol clnt_dg_create (which is provided by libtirpc)

autofs error log from /var/log/message:

Jul 23 12:11:58 ofire automount[25699]: open_mount:244: parse(sun): cannot open mount module
nfs (/usr/lib64/autofs/mount_nfs.so: undefined symbol: clnt_dg_create)
Jul 23 12:11:58 ofire automount[25699]: lookup(file): failed to open parse context

without this patch, mount_nfs.so fail link to libtirpc.so

 # ldd /usr/lib64/autofs/mount_nfs.so |grep tirpc

with this patch applied:

 # ldd /usr/lib64/autofs/mount_nfs.so |grep tirpc
        libtirpc.so.1 => /lib64/libtirpc.so.1 (0x00007f7d94920000)
---
 CHANGELOG        |    1 +
 modules/Makefile |    2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)
CHANGELOG +1 -0
index 296aaf8..679a469 100644
@@ -62,6 +62,7 @@
 - fix typo forced-shutdown should be force-shutdown.
 - fix hesiod check error and use correct $(LIBS) setting.
 - fix dead LDAP symbolic link when LDAP support is disabled.
+- add missing libtirpc lib to mount_nfs.so when TIRPC enabled.
 
 25/07/2012 autofs-5.0.7
 =======================
modules/Makefile +1 -1
index 8c0df18..e61e338 100644
@@ -116,6 +116,6 @@ lookup_ldap.so: lookup_ldap.c dclist.o base64.o $(SASL_OBJ)
 
 mount_nfs.so: mount_nfs.c replicated.o
 	$(CC) $(SOLDFLAGS) $(CFLAGS) -o mount_nfs.so \
-		mount_nfs.c replicated.o $(AUTOFS_LIB) $(LIBNSL)
+		mount_nfs.c replicated.o $(AUTOFS_LIB) $(LIBNSL) $(TIRPCLIB)
 	$(STRIP) mount_nfs.so
 
author maintainer
#13 lore raw

PATCH 11/25 autofs-5.0.7 - use compiler determined by configure instead of hard-coded ones

In reply to Ian Kent

From: Lan Yixun (dlan) <[email protected]>

the logic here is:
if user pass compiler variable, then use it, otherwise fallback to default value

this will provide most flexibility, so user can set this variable it would be even useful for cross-compile environment.

---
 CHANGELOG        |    1 +
 Makefile.conf.in |    3 +++
 Makefile.rules   |    8 ++++----
 3 files changed, 8 insertions(+), 4 deletions(-)
CHANGELOG +1 -0
index 679a469..8f349a2 100644
@@ -63,6 +63,7 @@
 - fix hesiod check error and use correct $(LIBS) setting.
 - fix dead LDAP symbolic link when LDAP support is disabled.
 - add missing libtirpc lib to mount_nfs.so when TIRPC enabled.
+- use compiler determined by configure instead of hard-coded ones.
 
 25/07/2012 autofs-5.0.7
 =======================
Makefile.conf.in +3 -0
index 3766d45..cb53e66 100644
@@ -109,3 +109,6 @@ initdir = @initdir@
 
 # Location of systemd unit files
 systemddir = @systemddir@
+
+# Use the compiler determined by configure instead of hard-coded ones
+CC := @CC@
Makefile.rules +4 -4
index 6b5b2bd..9aebd19 100644
@@ -38,10 +38,10 @@ STRIP     = strip --strip-debug
 endif
 endif
 
-CC        = gcc
-CXX       = g++
-CXXFLAGS  = $(CFLAGS)
-LD        = ld
+CC        ?= gcc
+CXX       ?= g++
+CXXFLAGS  ?= $(CFLAGS)
+LD        ?= ld
 SOLDFLAGS = -shared
 
 CFLAGS += -D_REENTRANT -D_FILE_OFFSET_BITS=64
author maintainer
#14 lore raw

PATCH 12/25 autofs-5.0.7 - remove hard-coded STRIP variable

In reply to Ian Kent

From: Lan Yixun (dlan) <[email protected]>

according to autoconf manual[1], build script should allow user override the CFLAGS, LDFLAGS varible (passed via ./configure). And actually, we should not directly use CFLAGS, LDFLAGS, but have to create another one (eg. AUTOFS_CFLAGS). FYI, see section "Variable: CFLAGS", the last paragraph.

Here, I slightly modify the code so that we can still fallback to the default one.

[1] http://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Preset-Output-Variables.html#Preset-Output-Variables

Edited by: Ian Kent <[email protected]>
- allow for previous LDFLAGS patch.
- rename patch to reflect what's actually done but leave the original description in place as a reminder.

---
 CHANGELOG      |    1 +
 Makefile.rules |    6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)
CHANGELOG +1 -0
index 8f349a2..ec9d7f0 100644
@@ -64,6 +64,7 @@
 - fix dead LDAP symbolic link when LDAP support is disabled.
 - add missing libtirpc lib to mount_nfs.so when TIRPC enabled.
 - use compiler determined by configure instead of hard-coded ones.
+- remove hard-coded STRIP variable.
 
 25/07/2012 autofs-5.0.7
 =======================
Makefile.rules +3 -3
index 9aebd19..d00ba3c 100644
@@ -25,16 +25,16 @@ endif
 ifdef DEBUG
 CFLAGS    ?= -g -Wall -DDEBUG
 LDFLAGS   ?= -g
-STRIP     = :
+STRIP     ?= :
 else
 ifdef DONTSTRIP
 CFLAGS    ?= -O2 -g
 LDFLAGS   ?= -g
-STRIP     = :
+STRIP     ?= :
 else
 CFLAGS    ?= -O2 -Wall
 LDFLAGS   ?= -s
-STRIP     = strip --strip-debug
+STRIP     ?= strip --strip-debug
 endif
 endif
 
author maintainer
#15 lore raw

PATCH 13/25 autofs-5.0.7 - use LIBS for link libraries

In reply to Ian Kent

From: Lan Yixun (dlan) <[email protected]>

quote:
"Don't use this variable to pass library names (-l) to the linker; use LIBS instead."

see the section "Variable: LDFLAGS" in:
http://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/PreseOutput-Variables.html#Preset-Output-Variables

Edited by: Ian Kent <[email protected]>
The variable usage changes for LDFLAGS and LIBS appear to be dependent on each other so merge them to try and avoid breakage if not all are used by someone. I also had changed the patch titles slightly when I imported them but they should be easily recognisable.
- merge patches
- pthread-link-library-should-be-put-into-LIBS-not-LDFLAGS
- library-should-be-passed-via-variable-LIBS-not-LDFLAGS
- accumulate-LIBS-variable-changes-otherwise-we-may-lose-early-settings
- allow-user-to-pass-LDFLAGS-via-configure
- add LIBS to build rules, might fix DMALLOC build problem

---
 CHANGELOG        |    1 +
 Makefile.rules   |   10 ++++++----
 daemon/Makefile  |    2 +-
 modules/Makefile |   12 ++++++------
 4 files changed, 14 insertions(+), 11 deletions(-)
CHANGELOG +1 -0
index ec9d7f0..3af7ece 100644
@@ -65,6 +65,7 @@
 - add missing libtirpc lib to mount_nfs.so when TIRPC enabled.
 - use compiler determined by configure instead of hard-coded ones.
 - remove hard-coded STRIP variable.
+- use LIBS for link libraries.
 
 25/07/2012 autofs-5.0.7
 =======================
Makefile.rules +6 -4
index d00ba3c..fa53459 100644
@@ -45,17 +45,19 @@ LD        ?= ld
 SOLDFLAGS = -shared
 
 CFLAGS += -D_REENTRANT -D_FILE_OFFSET_BITS=64
-LDFLAGS += -lpthread
+LIBS += -lpthread
 
 ifdef TIRPCLIB
 CFLAGS += -I/usr/include/tirpc
-LDFLAGS += $(TIRPCLIB)
+LIBS += $(TIRPCLIB)
 endif
 
 ifdef DMALLOCLIB
-LDFLAGS += $(DMALLOCLIB)
+LIBS += $(DMALLOCLIB)
 endif
 
+LIBS += $(LIBNSL)
+
 # Standard rules
 
 .SUFFIXES: .c .o .s .so
@@ -67,5 +69,5 @@ endif
 	$(CC) $(CFLAGS) -S $<
 
 .c.so:
-	$(CC) $(SOLDFLAGS) $(CFLAGS) -o $*.so $< $(AUTOFS_LIB) $(DMALLOCLIB) $(LIBNSL)
+	$(CC) $(LDFLAGS) $(SOLDFLAGS) $(CFLAGS) -o $*.so $< $(AUTOFS_LIB) $(LIBS)
 	$(STRIP) $*.so
daemon/Makefile +1 -1
index 9e9d635..6604c9a 100644
@@ -20,7 +20,7 @@ CFLAGS += -DAUTOFS_FIFO_DIR=\"$(autofsfifodir)\"
 CFLAGS += -DAUTOFS_FLAG_DIR=\"$(autofsflagdir)\"
 CFLAGS += -DVERSION_STRING=\"$(version)\"
 LDFLAGS += -rdynamic
-LIBS = -ldl
+LIBS += -ldl
 
 ifeq ($(LDAP), 1)
     CFLAGS += $(XML_FLAGS)
modules/Makefile +6 -6
index e61e338..de01ebd 100644
@@ -98,8 +98,8 @@ endif
 # Ad hoc compilation rules for modules which need auxilliary libraries
 #
 lookup_hesiod.so: lookup_hesiod.c
-	$(CC) $(SOLDFLAGS) $(CFLAGS) $(HESIOD_FLAGS) -o lookup_hesiod.so \
-		lookup_hesiod.c $(AUTOFS_LIB) $(LIBHESIOD) $(LIBRESOLV)
+	$(CC) $(LDFLAGS) $(SOLDFLAGS) $(CFLAGS) $(HESIOD_FLAGS) -o lookup_hesiod.so \
+		lookup_hesiod.c $(AUTOFS_LIB) $(LIBHESIOD) $(LIBRESOLV) $(LIBS)
 	$(STRIP) lookup_hesiod.so
 
 cyrus-sasl.o: cyrus-sasl.c
@@ -109,13 +109,13 @@ cyrus-sasl-extern.o: cyrus-sasl-extern.c
 	$(CC) $(CFLAGS) $(LDAP_FLAGS) -c $<
 
 lookup_ldap.so: lookup_ldap.c dclist.o base64.o $(SASL_OBJ)
-	$(CC) $(SOLDFLAGS) $(CFLAGS) $(LDAP_FLAGS) -o lookup_ldap.so \
+	$(CC) $(LDFLAGS) $(SOLDFLAGS) $(CFLAGS) $(LDAP_FLAGS) -o lookup_ldap.so \
 		lookup_ldap.c dclist.o base64.o $(SASL_OBJ) \
-		$(AUTOFS_LIB) $(LIBLDAP) $(LIBRESOLV)
+		$(AUTOFS_LIB) $(LIBLDAP) $(LIBRESOLV) $(LIBS)
 	$(STRIP) lookup_ldap.so
 
 mount_nfs.so: mount_nfs.c replicated.o
-	$(CC) $(SOLDFLAGS) $(CFLAGS) -o mount_nfs.so \
-		mount_nfs.c replicated.o $(AUTOFS_LIB) $(LIBNSL) $(TIRPCLIB)
+	$(CC) $(LDFLAGS) $(SOLDFLAGS) $(CFLAGS) -o mount_nfs.so \
+		mount_nfs.c replicated.o $(AUTOFS_LIB) $(LIBS)
 	$(STRIP) mount_nfs.so
 
author maintainer
#16 lore raw

PATCH 14/25 autofs-5.0.7 - unbundle NOTSTRIP from DEBUG so they dont depend on each other

In reply to Ian Kent

From: Lan Yixun (dlan) <[email protected]>

Gentoo build system expects unstripped elf image from the build system, seperate this, will allow us to better control the strip operation.

---
 CHANGELOG      |    1 +
 Makefile.rules |   10 ++++------
 2 files changed, 5 insertions(+), 6 deletions(-)
CHANGELOG +1 -0
index 3af7ece..dd6fc1a 100644
@@ -66,6 +66,7 @@
 - use compiler determined by configure instead of hard-coded ones.
 - remove hard-coded STRIP variable.
 - use LIBS for link libraries.
+- unbundle NOTSTRIP from DEBUG so they dont depend on each other.
 
 25/07/2012 autofs-5.0.7
 =======================
Makefile.rules +4 -6
index fa53459..0754a00 100644
@@ -25,18 +25,16 @@ endif
 ifdef DEBUG
 CFLAGS    ?= -g -Wall -DDEBUG
 LDFLAGS   ?= -g
-STRIP     ?= :
 else
+CFLAGS    ?= -O2 -Wall
+LDFLAGS   ?= -s
+endif
+
 ifdef DONTSTRIP
-CFLAGS    ?= -O2 -g
-LDFLAGS   ?= -g
 STRIP     ?= :
 else
-CFLAGS    ?= -O2 -Wall
-LDFLAGS   ?= -s
 STRIP     ?= strip --strip-debug
 endif
-endif
 
 CC        ?= gcc
 CXX       ?= g++
author maintainer
#17 lore raw

PATCH 15/25 autofs-5.0.7 - fix occasional build error when enable parallel compiling

In reply to Ian Kent

From: Lan Yixun (dlan) <[email protected]>

The problem here we pass "-jX" ( X> 1) to "make", and "rpc_subs.c" occasionally fail to compile due to it can't find "mount.h", the root cause it that "mount.h" haven't generated by that time, add this patch should fix this problem.
to make rpc_subs.o explicitly depend on "mount.h".

error message:

x86_64-pc-linux-gnu-gcc -O2 -pipe -march=core2 -D_REENTRANT -D_FILE_OFFSET_BITS=64 -I/usr/include/tirp c -D_REENTRANT -D_FILE_OFFSET_BITS=64 -I/usr/include/tirpc -I../include -fPIC -D_GNU_SOURCE -DAUTOFS_MA P_DIR=\"/etc/autofs\" -DAUTOFS_CONF_DIR=\"/etc/conf.d\" -c rpc_subs.c rpc_subs.c:46:19: fatal error: mount.h: No such file or directory

  #include "mount.h"
                   ^

compilation terminated.
make[1]: *** [rpc_subs.o] Error 1
make[1]: *** Waiting for unfinished
jobs....

---
 lib/Makefile |    2 ++
 1 file changed, 2 insertions(+)
lib/Makefile +2 -0
index 5418009..314779b 100644
@@ -70,6 +70,8 @@ nss_tok.o: nss_tok.c nss_parse.tab.h
 
 nss_parse.tab.o: nss_parse.tab.c nss_parse.tab.h
 
+rpc_subs.o: mount.h
+
 install: all
 
 clean:
author maintainer
#18 lore raw

PATCH 16/25 autofs-5.0.7 - fix compilation of lookup_ldap.c without sasl

In reply to Ian Kent

From: Dustin Polke <[email protected]>

See https://bugs.gentoo.org/show_bug.cgi?id=361899 for more info.

Edited by: Ian Kent <[email protected]>
- fix parse_ldap_config() is needed by ldap but previously excluded.
- exclude other references to ctxt->extern_cert and ctxt->extern_key.
- prevent memory leak if present in config but not used.
- remove now unused set_env().

---
 CHANGELOG             |    1 +
 include/lookup_ldap.h |    4 ++--
 lib/Makefile          |    4 ++++
 modules/Makefile      |    6 ++++--
 modules/lookup_ldap.c |   18 +++++++++++++-----
 5 files changed, 24 insertions(+), 9 deletions(-)
CHANGELOG +1 -0
index dd6fc1a..5764cb0 100644
@@ -67,6 +67,7 @@
 - remove hard-coded STRIP variable.
 - use LIBS for link libraries.
 - unbundle NOTSTRIP from DEBUG so they dont depend on each other.
+- fix compilation of lookup_ldap.c without sasl.
 
 25/07/2012 autofs-5.0.7
 =======================
include/lookup_ldap.h +2 -2
index 9a4ce73..f34c029 100644
@@ -11,6 +11,8 @@
 #include <krb5.h>
 #endif
 
+#include <libxml/tree.h>
+
 #include "list.h"
 #include "dclist.h"
 
@@ -92,7 +94,6 @@ struct lookup_context {
 };
 
 
-#ifdef WITH_SASL
 #define LDAP_AUTH_CONF_FILE "test"
 
 #define LDAP_TLS_DONT_USE	0
@@ -104,7 +105,6 @@ struct lookup_context {
 #define LDAP_AUTH_REQUIRED	0x0002
 #define LDAP_AUTH_AUTODETECT	0x0004
 #define LDAP_NEED_AUTH		(LDAP_AUTH_REQUIRED|LDAP_AUTH_AUTODETECT)
-#endif
 
 #define LDAP_AUTH_USESIMPLE	0x0008
 
lib/Makefile +4 -0
index 314779b..518b483 100644
@@ -24,6 +24,10 @@ CFLAGS += -I../include -fPIC -D_GNU_SOURCE
 CFLAGS += -DAUTOFS_MAP_DIR=\"$(autofsmapdir)\"
 CFLAGS += -DAUTOFS_CONF_DIR=\"$(autofsconfdir)\"
 
+ifeq ($(LDAP), 1)
+  CFLAGS += $(XML_FLAGS) $(XML_LIBS)
+endif
+
 .PHONY: all install clean
 
 all: autofs.a
modules/Makefile +4 -2
index de01ebd..8610783 100644
@@ -45,10 +45,12 @@ endif
 ifeq ($(LDAP), 1)
   SRCS += lookup_ldap.c
   MODS += lookup_ldap.so
+  LDAP_FLAGS += $(XML_FLAGS) -DLDAP_THREAD_SAFE
+  LIBLDAP += $(XML_LIBS)
   ifeq ($(SASL), 1)
     SASL_OBJ = cyrus-sasl.o cyrus-sasl-extern.o
-    LDAP_FLAGS += $(SASL_FLAGS) $(XML_FLAGS) $(KRB5_FLAGS) -DLDAP_THREAD_SAFE
-    LIBLDAP += $(LIBSASL) $(XML_LIBS) $(KRB5_LIBS)
+    LDAP_FLAGS += $(SASL_FLAGS) $(KRB5_FLAGS)
+    LIBLDAP += $(LIBSASL) $(KRB5_LIBS)
   endif
 endif
 
modules/lookup_ldap.c +13 -5
index 655e9fa..d05098f 100644
@@ -72,6 +72,7 @@ struct ldap_search_params {
 
 static int decode_percent_hack(const char *, char **);
 
+#ifdef WITH_SASL
 static int set_env(unsigned logopt, const char *name, const char *val)
 {
 	int ret = setenv(name, val, 1);
@@ -81,6 +82,7 @@ static int set_env(unsigned logopt, const char *name, const char *val)
 	}
 	return 1;
 }
+#endif
 
 #ifndef HAVE_LDAP_CREATE_PAGE_CONTROL
 int ldap_create_page_control(LDAP *ldap, ber_int_t pagesize,
@@ -205,9 +207,9 @@ int unbind_ldap_connection(unsigned logopt, LDAP *ldap, struct lookup_context *c
 {
 	int rv;
 
-#ifdef WITH_SASL
 	if (ctxt->use_tls == LDAP_TLS_RELEASE)
 		ctxt->use_tls = LDAP_TLS_INIT;
+#ifdef WITH_SASL
 	autofs_sasl_unbind(ctxt);
 #endif
 
@@ -265,7 +267,6 @@ LDAP *__init_ldap_connection(unsigned logopt, const char *uri, struct lookup_con
 		info(logopt, MODPREFIX "failed to set connection timeout to %d",
 		     net_timeout.tv_sec);
 
-#ifdef WITH_SASL
 	if (ctxt->use_tls) {
 		if (ctxt->version == 2) {
 			if (ctxt->tls_required) {
@@ -294,7 +295,6 @@ LDAP *__init_ldap_connection(unsigned logopt, const char *uri, struct lookup_con
 		}
 		ctxt->use_tls = LDAP_TLS_RELEASE;
 	}
-#endif
 
 	return ldap;
 }
@@ -618,10 +618,12 @@ static LDAP *do_connect(unsigned logopt, const char *uri, struct lookup_context
 {
 	LDAP *ldap;
 
+#ifdef WITH_SASL
 	if (ctxt->extern_cert && ctxt->extern_key) {
 		set_env(logopt, ENV_LDAPTLS_CERT, ctxt->extern_cert);
 		set_env(logopt, ENV_LDAPTLS_KEY, ctxt->extern_key);
 	}
+#endif
 
 	ldap = init_ldap_connection(logopt, uri, ctxt);
 	if (ldap) {
@@ -824,7 +826,6 @@ find_server:
 	return ldap;
 }
 
-#ifdef WITH_SASL
 int get_property(unsigned logopt, xmlNodePtr node, const char *prop, char **value)
 {
 	xmlChar *ret;
@@ -845,6 +846,7 @@ int get_property(unsigned logopt, xmlNodePtr node, const char *prop, char **valu
 	return 0;
 }
 
+#ifdef WITH_SASL
 /*
  *  For plain text, login and digest-md5 authentication types, we need
  *  user and password credentials.
@@ -857,6 +859,7 @@ int authtype_requires_creds(const char *authtype)
 		return 1;
 	return 0;
 }
+#endif
 
 /*
  *  Returns:
@@ -1089,6 +1092,7 @@ auth_fail:
 		}
 	} else if (auth_required == LDAP_AUTH_REQUIRED &&
 		  (authtype && !strncmp(authtype, "EXTERNAL", 8))) {
+#ifdef WITH_SASL
 		ret = get_property(logopt, root, "external_cert",  &extern_cert);
 		ret |= get_property(logopt, root, "external_key",  &extern_key);
 		/*
@@ -1107,6 +1111,7 @@ auth_fail:
 			if (extern_key)
 				free(extern_key);
 		}
+#endif
 	}
 
 	/*
@@ -1127,8 +1132,10 @@ auth_fail:
 	ctxt->secret = secret;
 	ctxt->client_princ = client_princ;
 	ctxt->client_cc = client_cc;
+#ifdef WITH_SASL
 	ctxt->extern_cert = extern_cert;
 	ctxt->extern_key = extern_key;
+#endif
 
 	debug(logopt, MODPREFIX
 	      "ldap authentication configured with the following options:");
@@ -1160,7 +1167,6 @@ out:
 
 	return ret;
 }
-#endif
 
 /*
  *  Take an input string as specified in the master map, and break it
@@ -1423,10 +1429,12 @@ static void free_context(struct lookup_context *ctxt)
 		defaults_free_searchdns(ctxt->sdns);
 	if (ctxt->dclist)
 		free_dclist(ctxt->dclist);
+#ifdef WITH_SASL
 	if (ctxt->extern_cert)
 		free(ctxt->extern_cert);
 	if (ctxt->extern_key)
 		free(ctxt->extern_key);
+#endif
 	free(ctxt);
 
 	return;
author maintainer
#19 lore raw

PATCH 17/25 autofs-5.0.7 - fix dumpmaps multi output

In reply to Ian Kent

If the maps contain the old style "multi" type the dumped map information isn't right.

---
 CHANGELOG    |    1 +
 lib/master.c |   61 +++++++++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 50 insertions(+), 12 deletions(-)
CHANGELOG +1 -0
index 5764cb0..5e58aca 100644
@@ -68,6 +68,7 @@
 - use LIBS for link libraries.
 - unbundle NOTSTRIP from DEBUG so they dont depend on each other.
 - fix compilation of lookup_ldap.c without sasl.
+- fix dumpmaps multi output.
 
 25/07/2012 autofs-5.0.7
 =======================
lib/master.c +49 -12

82 lines not on this page · raw message

author maintainer
#20 lore raw

PATCH 18/25 autofs-5.0.7 - try and cleanup after dumpmaps

In reply to Ian Kent

Try and cleanup a little after dumping maps.
It's not really necessary but but can help by reducing the noise from valgrind when checking code.

---
 CHANGELOG          |    1 +
 daemon/automount.c |   13 +++++++++++++
 lib/master.c       |    2 ++
 3 files changed, 16 insertions(+)
CHANGELOG +1 -0
index 5e58aca..1583c76 100644
@@ -69,6 +69,7 @@
 - unbundle NOTSTRIP from DEBUG so they dont depend on each other.
 - fix compilation of lookup_ldap.c without sasl.
 - fix dumpmaps multi output.
+- try and cleanup after dumpmaps.
 
 25/07/2012 autofs-5.0.7
 =======================
daemon/automount.c +13 -0
index 1d0b64e..6849272 100644
@@ -2136,6 +2136,8 @@ int main(int argc, char *argv[])
 	}
 
 	if (dumpmaps) {
+		struct master_mapent *entry;
+		struct list_head *head, *p;
 		struct mapent_cache *nc;
 
 		open_log();
@@ -2152,6 +2154,17 @@ int main(int argc, char *argv[])
 
 		lookup_nss_read_master(master_list, 0);
 		master_show_mounts(master_list);
+
+		head = &master_list->mounts;
+		p = head->next;
+		while (p != head) {
+			entry = list_entry(p, struct master_mapent, list);
+			p = p->next;
+			master_free_mapent_sources(entry, 1);
+			master_free_mapent(entry);
+		}
+		master_kill(master_list);
+
 		exit(0);
 	}
 
lib/master.c +2 -0
index df6c141..734485e 100644
@@ -1426,6 +1426,8 @@ int master_show_mounts(struct master *master)
 			source = source->next;
 		}
 
+		lookup_close_lookup(ap);
+
 		printf("\n");
 	}
 
author maintainer
#21 lore raw

PATCH 19/25 autofs-5.0.7 - teach dumpmaps to output simple key value pairs

In reply to Ian Kent

The dumpmaps option doesn't allow maps to be output in <key, value> pairs suitable for use as a file map.

This could be useful to save current maps as a backup for emergency use.

If the dumpmaps option is given and is followed by two parameters, "<map type> <map name>" then simple <key, value> pairs that would be read in by a map read are printed to stdout if the given map type and map name are found in the map configuration.

If the map is an LDAP map and there is more than one map of same name in different base dns only the first map encountered by autofs will be listed.

If the map type is an old style multi-map and any one of the map names in the multi-map entry matches the given map name the entries that would be used by autofs for the whole multi-map will be listed.

---
 CHANGELOG          |    1 
 daemon/automount.c |   51 +++++++++++---
 include/master.h   |    1 
 lib/master.c       |  187 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 man/automount.8    |   18 ++++-
 5 files changed, 243 insertions(+), 15 deletions(-)
CHANGELOG +1 -0
index 1583c76..07f9d6d 100644
@@ -70,6 +70,7 @@
 - fix compilation of lookup_ldap.c without sasl.
 - fix dumpmaps multi output.
 - try and cleanup after dumpmaps.
+- teach dumpmaps to output simple key value pairs.
 
 25/07/2012 autofs-5.0.7
 =======================
daemon/automount.c +38 -13

80 lines not on this page · raw message

include/master.h +1 -0
index 824707c..e77d5ba 100644
@@ -112,6 +112,7 @@ int master_submount_list_empty(struct autofs_point *ap);
 int master_notify_submount(struct autofs_point *, const char *path, enum states);
 void master_notify_state_change(struct master *, int);
 int master_mount_mounts(struct master *, time_t, int);
+int dump_map(struct master *, const char *, const char *);
 int master_show_mounts(struct master *);
 extern inline unsigned int master_get_logopt(void);
 int master_list_empty(struct master *);
lib/master.c +187 -0

193 lines not on this page · raw message

man/automount.8 +16 -2
index 844b876..48f5c5a 100644
@@ -57,8 +57,22 @@ Run the daemon in the foreground and log to stderr instead of syslog."
 Enables the use of ramdom selection when choosing a host from a
 list of replicated servers.
 .TP
-.I "\-m, \-\-dumpmaps"
-Dump configured automounter maps, then exit.
+.I "\-m, \-\-dumpmaps [<map type> <map name]"
+With no parameters, list information about the configured automounter
+maps, then exit.
+
+If the dumpmaps option is given and is followed by two parameters,
+"<map type> <map name>" then simple "<key, value>" pairs that would
+be read in by a map read are printed to stdout if the given map type
+and map name are found in the map configuration.
+
+If the map is an LDAP map and there is more than one map of same name
+in different base dns only the first map encountered by autofs will
+be listed.
+
+If the map type is an old style multi-map and any one of the map
+names in the multi-map entry matches the given map name the entries
+that would be used by autofs for the whole multi-map will be listed.
 .TP
 .I "\-O, \-\-global-options"
 Allows the specification of global mount options used for all master
author maintainer
#22 lore raw

PATCH 20/25 autofs-5.0.7 - fix syncronize handle_mounts() shutdown

In reply to Ian Kent

A recent change to fix a problem where automount would exit when re-reading the master map if more than on entry had been removed causes automount to ignore HUP signals if the mount list is empty.

Removing that check and testing shows that the remainder of the change still fixes the original problem.

---
 CHANGELOG          |    1 +
 daemon/automount.c |    2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)
CHANGELOG +1 -0
index 07f9d6d..618ac20 100644
@@ -71,6 +71,7 @@
 - fix dumpmaps multi output.
 - try and cleanup after dumpmaps.
 - teach dumpmaps to output simple key value pairs.
+- fix syncronize handle_mounts() shutdown.
 
 25/07/2012 autofs-5.0.7
 =======================
daemon/automount.c +1 -1
index 2ce49bf..97c726a 100644
@@ -1286,7 +1286,7 @@ static int do_hup_signal(struct master *master, time_t age)
 
 	master_mutex_lock();
 	/* Already doing a map read or shutdown or no mounts */
-	if (master->reading || list_empty(&master->mounts)) {
+	if (master->reading) {
 		status = pthread_mutex_unlock(&mrc.mutex);
 		if (status)
 			fatal(status);
author maintainer
#23 lore raw

PATCH 21/25 autofs-5.0.7 - fix fix wildcard multi map regression

In reply to Ian Kent

A recent patch to fix a wildcard multi map mount regression has a side effect of causing a deadlock at startup when trying to re-connect to existing mounts.

The patch required the map entry cache write lock be taken so the cache could be updated. But when starting and trying to re-connect to existing mounts there's no need to update the cache.

---
 CHANGELOG                |    1 +
 modules/lookup_file.c    |   25 ++++++++++++++++++++-----
 modules/lookup_ldap.c    |   23 +++++++++++++++++++----
 modules/lookup_nisplus.c |   26 +++++++++++++++++++++-----
 modules/lookup_sss.c     |   22 ++++++++++++++++++----
 modules/lookup_yp.c      |   23 +++++++++++++++++++----
 6 files changed, 98 insertions(+), 22 deletions(-)
CHANGELOG +1 -0
index 618ac20..c3f12e5 100644
@@ -72,6 +72,7 @@
 - try and cleanup after dumpmaps.
 - teach dumpmaps to output simple key value pairs.
 - fix syncronize handle_mounts() shutdown.
+- fix fix wildcard multi map regression.
 
 25/07/2012 autofs-5.0.7
 =======================
modules/lookup_file.c +20 -5
index 4b4ee89..83ef048 100644
@@ -1042,7 +1042,7 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 			return NSS_STATUS_UNAVAIL;
 		}
 
-		cache_writelock(mc);
+		cache_readlock(mc);
 		me = cache_lookup_first(mc);
 		if (me && st.st_mtime <= me->age) {
 			/*
@@ -1084,7 +1084,18 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 		}
 	}
 
-	cache_writelock(mc);
+	/*
+	 * We can't take the writelock for direct mounts. If we're
+	 * starting up or trying to re-connect to an existing direct
+	 * mount we could be iterating through the map entries with
+	 * the readlock held. But we don't need to update the cache
+	 * when we're starting up so just take the readlock in that
+	 * case.
+	 */
+	if (ap->flags & MOUNT_FLAG_REMOUNT)
+		cache_readlock(mc);
+	else
+		cache_writelock(mc);
 do_cache_lookup:
 	me = cache_lookup(mc, key);
 	/*
@@ -1102,10 +1113,11 @@ do_cache_lookup:
 	}
 	if (me && me->mapent) {
 		/*
-		 * Add wildcard match for later validation checks and
-		 * negative cache lookups.
+		 * If this is a lookup add wildcard match for later validation
+		 * checks and negative cache lookups.
 		 */
-		if (ap->type == LKP_INDIRECT && *me->key == '*') {
+		if (!(ap->flags & MOUNT_FLAG_REMOUNT) &&
+		    ap->type == LKP_INDIRECT && *me->key == '*') {
 			ret = cache_update(mc, source, key, me->mapent, me->age);
 			if (!(ret & (CHE_OK | CHE_UPDATED)))
 				me = NULL;
@@ -1130,6 +1142,9 @@ do_cache_lookup:
 	ret = ctxt->parse->parse_mount(ap, key, key_len,
 				       mapent, ctxt->parse->context);
 	if (ret) {
+		/* Don't update negative cache when re-connecting */
+		if (ap->flags & MOUNT_FLAG_REMOUNT)
+			return NSS_STATUS_TRYAGAIN;
 		cache_writelock(mc);
 		cache_update_negative(mc, source, key, ap->negative_timeout);
 		cache_unlock(mc);
modules/lookup_ldap.c +19 -4
index d05098f..2ab1e8c 100644
@@ -3016,7 +3016,18 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 			return status;
 	}
 
-	cache_writelock(mc);
+	/*
+	 * We can't take the writelock for direct mounts. If we're
+	 * starting up or trying to re-connect to an existing direct
+	 * mount we could be iterating through the map entries with
+	 * the readlock held. But we don't need to update the cache
+	 * when we're starting up so just take the readlock in that
+	 * case.
+	 */
+	if (ap->flags & MOUNT_FLAG_REMOUNT)
+		cache_readlock(mc);
+	else
+		cache_writelock(mc);
 	me = cache_lookup(mc, key);
 	/* Stale mapent => check for entry in alternate source or wildcard */
 	if (me && !me->mapent) {
@@ -3028,10 +3039,11 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	}
 	if (me && me->mapent) {
 		/*
-		 * Add wildcard match for later validation checks and
-		 * negative cache lookups.
+		 * If this is a lookup add wildcard match for later validation
+		 * checks and negative cache lookups.
 		 */
-		if (ap->type == LKP_INDIRECT && *me->key == '*') {
+		if (!(ap->flags & MOUNT_FLAG_REMOUNT) &&
+		    ap->type == LKP_INDIRECT && *me->key == '*') {
 			ret = cache_update(mc, source, key, me->mapent, me->age);
 			if (!(ret & (CHE_OK | CHE_UPDATED)))
 				me = NULL;
@@ -3053,6 +3065,9 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	ret = ctxt->parse->parse_mount(ap, key, key_len,
 				       mapent, ctxt->parse->context);
 	if (ret) {
+		/* Don't update negative cache when re-connecting */
+		if (ap->flags & MOUNT_FLAG_REMOUNT)
+			return NSS_STATUS_TRYAGAIN;
 		cache_writelock(mc);
 		cache_update_negative(mc, source, key, ap->negative_timeout);
 		cache_unlock(mc);
modules/lookup_nisplus.c +21 -5
index ef942a7..08878bb 100644
@@ -561,7 +561,18 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 			return status;
 	}
 
-	cache_writelock(mc);
+	/*
+	 * We can't take the writelock for direct mounts. If we're
+	 * starting up or trying to re-connect to an existing direct
+	 * mount we could be iterating through the map entries with
+	 * the readlock held. But we don't need to update the cache
+	 * when we're starting up so just take the readlock in that
+	 * case.
+	 */
+	if (ap->flags & MOUNT_FLAG_REMOUNT)
+		cache_readlock(mc);
+	else
+		cache_writelock(mc);
 	me = cache_lookup(mc, key);
 	/* Stale mapent => check for entry in alternate source or wildcard */
 	if (me && !me->mapent) {
@@ -573,10 +584,11 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	}
 	if (me && me->mapent) {
 		/*
-		 * Add wildcard match for later validation checks and
-		 * negative cache lookups.
+		 * If this is a lookup add wildcard match for later validation
+		 * checks and negative cache lookups.
 		 */
-		if (ap->type == LKP_INDIRECT && *me->key == '*') {
+		if (!(ap->flags & MOUNT_FLAG_REMOUNT) &&
+		    ap->type == LKP_INDIRECT && *me->key == '*') {
 			ret = cache_update(mc, source, key, me->mapent, me->age);
 			if (!(ret & (CHE_OK | CHE_UPDATED)))
 				me = NULL;
@@ -603,6 +615,11 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 		time_t now = time(NULL);
 		int rv = CHE_OK;
 
+		free(mapent);
+
+		/* Don't update negative cache when re-connecting */
+		if (ap->flags & MOUNT_FLAG_REMOUNT)
+			return NSS_STATUS_TRYAGAIN;
 		cache_writelock(mc);
 		me = cache_lookup_distinct(mc, key);
 		if (!me)
@@ -612,7 +629,6 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 			me->status = time(NULL) + ap->negative_timeout;
 		}
 		cache_unlock(mc);
-		free(mapent);
 		return NSS_STATUS_TRYAGAIN;
 	}
 	free(mapent);
modules/lookup_sss.c +18 -4
index 1fe740b..4a9cfd2 100644
@@ -635,7 +635,17 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 			return status;
 	}
 
-	cache_readlock(mc);
+	/*
+	 * We can't take the writelock for direct mounts. If we're
+	 * starting up or trying to re-connect to an existing direct
+	 * mount we could be iterating through the map entries with
+	 * the readlock held. But we don't need to update the cache
+	 * when we're starting up so just take the readlock in that
+	 */
+	if (ap->flags & MOUNT_FLAG_REMOUNT)
+		cache_writelock(mc);
+	else
+		cache_readlock(mc);
 	me = cache_lookup(mc, key);
 	/* Stale mapent => check for entry in alternate source or wildcard */
 	if (me && !me->mapent) {
@@ -647,10 +657,11 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	}
 	if (me && me->mapent) {
 		/*
-		 * Add wildcard match for later validation checks and
-		 * negative cache lookups.
+		 * If this is a lookup add wildcard match for later validation
+		 * checks and negative cache lookups.
 		 */
-		if (ap->type == LKP_INDIRECT && *me->key == '*') {
+		if (ap->type == LKP_INDIRECT && *me->key == '*' &&
+		   !(ap->flags & MOUNT_FLAG_REMOUNT)) {
 			ret = cache_update(mc, source, key, me->mapent, me->age);
 			if (!(ret & (CHE_OK | CHE_UPDATED)))
 				me = NULL;
@@ -672,6 +683,9 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	ret = ctxt->parse->parse_mount(ap, key, key_len,
 				       mapent, ctxt->parse->context);
 	if (ret) {
+		/* Don't update negative cache when re-connecting */
+		if (ap->flags & MOUNT_FLAG_REMOUNT)
+			return NSS_STATUS_TRYAGAIN;
 		cache_writelock(mc);
 		cache_update_negative(mc, source, key, ap->negative_timeout);
 		cache_unlock(mc);
modules/lookup_yp.c +19 -4
index e99e3c0..4d1848e 100644
@@ -662,7 +662,18 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 			return status;
 	}
 
-	cache_writelock(mc);
+	/*
+	 * We can't take the writelock for direct mounts. If we're
+	 * starting up or trying to re-connect to an existing direct
+	 * mount we could be iterating through the map entries with
+	 * the readlock held. But we don't need to update the cache
+	 * when we're starting up so just take the readlock in that
+	 * case.
+	 */
+	if (ap->flags & MOUNT_FLAG_REMOUNT)
+		cache_readlock(mc);
+	else
+		cache_writelock(mc);
 	me = cache_lookup(mc, key);
 	/* Stale mapent => check for entry in alternate source or wildcard */
 	if (me && !me->mapent) {
@@ -674,10 +685,11 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 	}
 	if (me && me->mapent) {
 		/*
-		 * Add wildcard match for later validation checks and
-		 * negative cache lookups.
+		 * If this is a lookup add wildcard match for later validation
+		 * checks and negative cache lookups.
 		 */
-		if (ap->type == LKP_INDIRECT && *me->key == '*') {
+		if (ap->type == LKP_INDIRECT && *me->key == '*' &&
+		   !(ap->flags & MOUNT_FLAG_REMOUNT)) {
 			ret = cache_update(mc, source, key, me->mapent, me->age);
 			if (!(ret & (CHE_OK | CHE_UPDATED)))
 				me = NULL;
@@ -698,6 +710,9 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 		ret = ctxt->parse->parse_mount(ap, key, key_len,
 					       mapent, ctxt->parse->context);
 		if (ret) {
+			/* Don't update negative cache when re-connecting */
+			if (ap->flags & MOUNT_FLAG_REMOUNT)
+				return NSS_STATUS_TRYAGAIN;
 			cache_writelock(mc);
 			cache_update_negative(mc, source, key, ap->negative_timeout);
 			cache_unlock(mc);
author maintainer
#24 lore raw

PATCH 22/25 autofs-5.0.7 - improve timeout option description

In reply to Ian Kent

The description of the configuration option is a little misleading. So add some explaination about the internal default and the reason the default installed configuration overrides it.

---
 CHANGELOG                      |    1 +
 man/auto.master.5.in           |    5 ++++-
 man/automount.8                |    5 ++++-
 redhat/autofs.sysconfig.in     |    5 ++++-
 samples/autofs.conf.default.in |    5 ++++-
 5 files changed, 17 insertions(+), 4 deletions(-)
CHANGELOG +1 -0
index c3f12e5..9639703 100644
@@ -73,6 +73,7 @@
 - teach dumpmaps to output simple key value pairs.
 - fix syncronize handle_mounts() shutdown.
 - fix fix wildcard multi map regression.
+- improve timeout option description.
 
 25/07/2012 autofs-5.0.7
 =======================
man/auto.master.5.in +4 -1
index 7907ffa..65a86b2 100644
@@ -212,7 +212,10 @@ configuration file
 They are:
 .TP
 .B TIMEOUT
-sets the default mount timeout (program default 600).
+Sets the default mount timeout in seconds. The internal program
+default is 10 minutes, but the default installed configuration
+overrides this and sets the timeout to 5 minutes to be consistent
+with earlier autofs releases.
 .TP
 .B NEGATIVE_TIMEOUT
 Set the default timeout for caching failed key lookups (program default
man/automount.8 +4 -1
index 48f5c5a..abb40bc 100644
@@ -29,10 +29,13 @@ Print brief help on program usage.
 .I "\-p, \-\-pid-file"
 Write the pid of the daemon to the specified file.
 .TP
-.I "\-t, \-\-timeout"
+.I "\-t <seconds>, \-\-timeout <seconds>"
 Set the global minimum timeout, in seconds, until directories
 are unmounted. The default is 10 minutes. Setting the timeout
 to zero disables umounts completely.
+The internal program default is 10 minutes, but the default
+installed configuration overrides this and sets the timeout
+to 5 minutes to be consistent with earlier autofs releases.
 .TP
 .I "\-n <seconds>, \-\-negative\-timeout <seconds>"
 Set the default timeout for caching failed key lookups. The default is 60 seconds.
redhat/autofs.sysconfig.in +4 -1
index a8992c4..9d8d704 100644
@@ -5,7 +5,10 @@
 #
 #MASTER_MAP_NAME="auto.master"
 #
-# TIMEOUT - set the default mount timeout (default 600).
+# TIMEOUT - set the default mount timeout in secons. The internal
+#	    program default is 10 minutes, but the default installed
+#	    configuration overrides this and sets the timeout to 5
+#	    minutes to be consistent with earlier autofs releases.
 #
 TIMEOUT=300
 #
samples/autofs.conf.default.in +4 -1
index 1da89cf..695b010 100644
@@ -5,7 +5,10 @@
 #
 #MASTER_MAP_NAME="auto.master"
 #
-# TIMEOUT - set the default mount timeout (default 600).
+# TIMEOUT - set the default mount timeout in secons. The internal
+#	    program default is 10 minutes, but the default installed
+#	    configuration overrides this and sets the timeout to 5
+#	    minutes to be consistent with earlier autofs releases.
 #
 TIMEOUT=300
 #
author maintainer
#25 lore raw

PATCH 23/25 autofs-5.0.7 - only probe specific nfs version when requested

In reply to Ian Kent

If a specific NFS version is given as an option the set the probe flags to probe only that version.

---
 CHANGELOG           |    1 +
 modules/mount_nfs.c |   10 +++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)
CHANGELOG +1 -0
index 9639703..2499f1c 100644
@@ -74,6 +74,7 @@
 - fix syncronize handle_mounts() shutdown.
 - fix fix wildcard multi map regression.
 - improve timeout option description.
+- only probe specific nfs version if requested.
 
 25/07/2012 autofs-5.0.7
 =======================
modules/mount_nfs.c +9 -1
index adf2002..5a582ef 100644
@@ -146,7 +146,15 @@ int mount_mount(struct autofs_point *ap, const char *root, const char *name, int
 				if (strncmp("vers=4", cp, o_len) == 0 ||
 				    strncmp("nfsvers=4", cp, o_len) == 0)
 					vers = NFS4_VERS_MASK | TCP_SUPPORTED;
-				else if (strstr(cp, "port=") == cp &&
+				else if (strncmp("vers=3", cp, o_len) == 0 ||
+					 strncmp("nfsvers=3", cp, o_len) == 0) {
+					vers &= ~(NFS4_VERS_MASK | NFS_VERS_MASK);
+					vers |= NFS3_REQUESTED;
+				} else if (strncmp("vers=2", cp, o_len) == 0 ||
+					 strncmp("nfsvers=2", cp, o_len) == 0) {
+					vers &= ~(NFS4_VERS_MASK | NFS_VERS_MASK);
+					vers |= NFS2_REQUESTED;
+				} else if (strstr(cp, "port=") == cp &&
 					 o_len - 5 < 25) {
 					char optport[25];
 
author maintainer
#26 lore raw

PATCH 24/25 autofs-5.0.7 - fix bad mkdir permission on create

In reply to Ian Kent

Reported by Gordon Lack (gordon[dot]m[dot]lack[at]gsk[dot]com).

If the automount daemon needs to create a directory (hierarchy) for an automount and it is started up with a umask of 027 (or similar) then it creates unusable directories (permission == 550).

---
 CHANGELOG          |    1 +
 daemon/automount.c |    5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)
CHANGELOG +1 -0
index 2499f1c..3a3fec1 100644
@@ -75,6 +75,7 @@
 - fix fix wildcard multi map regression.
 - improve timeout option description.
 - only probe specific nfs version if requested.
+- fix bad mkdir permission on create.
 
 25/07/2012 autofs-5.0.7
 =======================
daemon/automount.c +4 -1
index 97c726a..6b55103 100644
@@ -122,7 +122,10 @@ static int do_mkdir(const char *parent, const char *path, mode_t mode)
 		status = statfs(parent, &fs);
 	if ((status != -1 && fs.f_type == (__SWORD_TYPE) AUTOFS_SUPER_MAGIC) ||
 	    contained_in_local_fs(path)) {
-		if (mkdir(path, mode) == -1) {
+		int mask = umask(0022);
+		int ret = mkdir(path, mode);
+		void umask(mask);
+		if (ret == -1) {
 			errno = EACCES;
 			return 0;
 		}
author maintainer
#27 lore raw

Re: [PATCH 24/25] autofs-5.0.7 - fix bad mkdir permission on create

Quoted 27 lines · Ian Kent

Reported by Gordon Lack (gordon[dot]m[dot]lack[at]gsk[dot]com).

If the automount daemon needs to create a directory (hierarchy) for an automount and it is started up with a umask of 027 (or similar) then it creates unusable directories (permission == 550).

---
 CHANGELOG          |    1 +
 daemon/automount.c |    5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)
CHANGELOG +1 -0
index 2499f1c..3a3fec1 100644
@@ -75,6 +75,7 @@
 - fix fix wildcard multi map regression.
 - improve timeout option description.
 - only probe specific nfs version if requested.
+- fix bad mkdir permission on create.
 
 25/07/2012 autofs-5.0.7
 =======================
daemon/automount.c +0 -0
index 97c726a..6b55103 100644

daemon/automount.c static int do_mkdir(const char *parent, const char *path, mode_t mode) · line 127

Quoted 7 lines
daemon/automount.c +3 -1
@@ -122,7 +122,10 @@ static int do_mkdir(const char *parent, const char *path, mode_t mode)
 		status = statfs(parent, &fs);
 	if ((status != -1 && fs.f_type == (__SWORD_TYPE) AUTOFS_SUPER_MAGIC) ||
 	    contained_in_local_fs(path)) {
-		if (mkdir(path, mode) == -1) {
+		int mask = umask(0022);
+		int ret = mkdir(path, mode);
+		void umask(mask);

OK, so that void should be (void).

Quoted 9 lines
daemon/automount.c +1 -1
+		if (ret == -1) {
 			errno = EACCES;
 			return 0;
 		}
 
--

To unsubscribe from this list: send the line "unsubscribe autofs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author maintainer
#28 lore raw

PATCH 25/25 autofs-5.0.7 - setup program map env from macro table

In reply to Ian Kent

The ability to pass parameters to program maps, in some way, is needed. Standard autofs specifies that program maps have one argument so passing parameters as arguments shouldn't be done.

This patch sets the existing macro table definitions (for both global and local table) as environment variables before calling the map. The values are not checked after return so, at this stage, program maps can't change macro definitions.

---
 include/macros.h         |    1 +
 lib/macros.c             |   28 ++++++++++++++++++++++++++++
 modules/lookup_program.c |   20 ++++++++++++++++++++
 3 files changed, 49 insertions(+)
include/macros.h +1 -0
index a73a4a7..5077b5d 100644
@@ -40,5 +40,6 @@ void macro_free_global_table(void);
 void macro_free_table(struct substvar *table);
 const struct substvar *
 macro_findvar(const struct substvar *table, const char *str, int len);
+void macro_setenv(const struct substvar *table);
 
 #endif
lib/macros.c +28 -0
index 32b70bf..33c2ada 100644
@@ -421,3 +421,31 @@ macro_findvar(const struct substvar *table, const char *str, int len)
 	return NULL;
 }
 
+/* Set environment from macro variable table */
+void macro_setenv(const struct substvar *table)
+{
+	const struct substvar *sv = system_table;
+	const struct substvar *lv = table;
+
+	/*
+	 * First set environment from global table, matching local
+	 * variables will overwrite these.
+	 */
+	while (sv) {
+		if (sv->def)
+			setenv(sv->def, sv->val, 1);
+		sv = sv->next;
+	}
+
+	error(LOGOPT_ANY, "table %p", table);
+	dump_table(table);
+
+	/* Next set environment from the local table */
+	while (lv) {
+		if (lv->def)
+			setenv(lv->def, lv->val, 1);
+		lv = lv->next;
+	}
+
+	return;
+}
modules/lookup_program.c +20 -0
index 2457108..7e22b38 100644
@@ -36,9 +36,17 @@
 
 struct lookup_context {
 	const char *mapname;
+	char *mapfmt;
 	struct parse_mod *parse;
 };
 
+struct parse_context {
+	char *optstr;		/* Mount options */
+	char *macros;		/* Map wide macro defines */
+	struct substvar *subst;	/* $-substitutions */
+	int slashify_colons;	/* Change colons to slashes? */
+};
+
 int lookup_version = AUTOFS_LOOKUP_VERSION;	/* Required by protocol */
 
 int lookup_init(const char *mapfmt, int argc, const char *const *argv, void **context)
@@ -79,6 +87,8 @@ int lookup_init(const char *mapfmt, int argc, const char *const *argv, void **co
 	if (!mapfmt)
 		mapfmt = MAPFMT_DEFAULT;
 
+	ctxt->mapfmt = strdup(mapfmt);
+
 	ctxt->parse = open_parse(mapfmt, MODPREFIX, argc - 1, argv + 1);
 	if (!ctxt->parse) {
 		logmsg(MODPREFIX "failed to open parse context");
@@ -255,6 +265,14 @@ int lookup_mount(struct autofs_point *ap, const char *name, int name_len, void *
 			warn(ap->logopt,
 			     MODPREFIX "failed to set PWD to %s for map %s",
 			     ap->path, ctxt->mapname);
+		/*
+		 * MAPFMT_DEFAULT must be "sun" for ->parse_init() to have setup
+		 * the macro table.
+		 */
+		if (ctxt->mapfmt && strcmp(ctxt->mapfmt, "MAPFMT_DEFAULT")) {
+			struct parse_context *pctxt = (struct parse_context *) ctxt->parse->context;
+			macro_setenv(pctxt->subst);
+		}
 		execl(ctxt->mapname, ctxt->mapname, name, NULL);
 		_exit(255);	/* execl() failed */
 	}
@@ -448,6 +466,8 @@ int lookup_done(void *context)
 {
 	struct lookup_context *ctxt = (struct lookup_context *) context;
 	int rv = close_parse(ctxt->parse);
+	if (ctxt->mapfmt)
+		free(ctxt->mapfmt);
 	free(ctxt);
 	return rv;
 }
#29 lore raw
In reply to Ian Kent
Quoted 77 lines · Ian Kent

There are patches from several contributors I plan on committing so I thought posting them to the list for final review would be sensible.

I'm not sure when I'll commit the changes but I hope in the next couple of days.

---

Dustin Polke (1):
autofs-5.0.7 - fix compilation of lookup_ldap.c without sasl

Ian Kent (13):
autofs-5.0.7 - don't override LDFLAGS in make rules autofs-5.0.7 - fix a couple of compiler warnings autofs-5.0.7 - add after sssd dependency to unit file autofs-5.0.7 - dont start readmap unless ready autofs-5.0.7 - fix dumpmaps multi output autofs-5.0.7 - try and cleanup after dumpmaps autofs-5.0.7 - teach dumpmaps to output simple key value pairs autofs-5.0.7 - fix syncronize handle_mounts() shutdown autofs-5.0.7 - fix fix wildcard multi map regression autofs-5.0.7 - improve timeout option description autofs-5.0.7 - only probe specific nfs version when requested autofs-5.0.7 - fix bad mkdir permission on create autofs-5.0.7 - setup program map env from macro table

Lan Yixun (dlan) (10):
autofs-5.0.7 - fix compile error with heimdal support enabled autofs-5.0.7 - fix typo forced-shutdown should be force-shutdown autofs-5.0.7 - fix hesiod check error and use correct $(LIBS) setting autofs-5.0.7 - fix dead LDAP symbolic link when LDAP support is disabled autofs-5.0.7 - add missing libtirpc lib to mount_nfs.so when TIRPC enabled autofs-5.0.7 - use compiler determined by configure instead of hard-coded ones autofs-5.0.7 - remove hard-coded STRIP variable
autofs-5.0.7 - use LIBS for link libraries
autofs-5.0.7 - unbundle NOTSTRIP from DEBUG so they dont depend on each other autofs-5.0.7 - fix occasional build error when enable parallel compiling

Leonardo Chiquitto (1):
autofs-5.0.7 - fix crash due to thread unsafe use of libldap

 CHANGELOG                      |   23 ++++
 Makefile.conf.in               |    3
 Makefile.rules                 |   34 +++--
 aclocal.m4                     |   19 ++-
 autofs.spec                    |    3
 configure                      |  104 +++++++++++++++--
 configure.in                   |    2
 daemon/Makefile                |    2
 daemon/automount.c             |   71 +++++++++--
 daemon/state.c                 |   15 +-
 include/config.h.in            |    3
 include/lookup_ldap.h          |    4 -
 include/macros.h               |    1
 include/master.h               |    1
 lib/Makefile                   |    6 +
 lib/macros.c                   |   28 ++++
 lib/master.c                   |  250 ++++++++++++++++++++++++++++++++++++++--
 man/auto.master.5.in           |    5 +
 man/automount.8                |   23 +++-
 modules/Makefile               |   20 ++-
 modules/cyrus-sasl.c           |   39 +++++-
 modules/lookup_file.c          |   25 +++-
 modules/lookup_ldap.c          |   76 +++++++++++-
 modules/lookup_nisplus.c       |   26 +++-
 modules/lookup_program.c       |   20 +++
 modules/lookup_sss.c           |   22 +++-
 modules/lookup_yp.c            |   23 +++-
 modules/mount_bind.c           |    2
 modules/mount_nfs.c            |   14 ++
 redhat/autofs.sysconfig.in     |    5 +
 samples/autofs.conf.default.in |    5 +
 samples/autofs.service.in      |    2
 32 files changed, 753 insertions(+), 123 deletions(-)
Quoted 2 lines

--
Ian

HI Ian:
May I suggest you pushing this patch serials to one test branch
(for-next?) in the public repository?
I find this much more convenient for me to test (instead of fetch all the patches and apply myself).
Since this branch is experimental, you can delete it later or rebase to master branch.

Dennis Lan (dlan)

author maintainer
#30 lore raw
Quoted 82 lines · Dennis Lan (dlan)
Quoted 77 lines · Ian Kent

There are patches from several contributors I plan on committing so I thought posting them to the list for final review would be sensible.

I'm not sure when I'll commit the changes but I hope in the next couple of days.

---

Dustin Polke (1):
autofs-5.0.7 - fix compilation of lookup_ldap.c without sasl

Ian Kent (13):
autofs-5.0.7 - don't override LDFLAGS in make rules autofs-5.0.7 - fix a couple of compiler warnings autofs-5.0.7 - add after sssd dependency to unit file autofs-5.0.7 - dont start readmap unless ready autofs-5.0.7 - fix dumpmaps multi output autofs-5.0.7 - try and cleanup after dumpmaps autofs-5.0.7 - teach dumpmaps to output simple key value pairs autofs-5.0.7 - fix syncronize handle_mounts() shutdown autofs-5.0.7 - fix fix wildcard multi map regression autofs-5.0.7 - improve timeout option description autofs-5.0.7 - only probe specific nfs version when requested autofs-5.0.7 - fix bad mkdir permission on create autofs-5.0.7 - setup program map env from macro table

Lan Yixun (dlan) (10):
autofs-5.0.7 - fix compile error with heimdal support enabled autofs-5.0.7 - fix typo forced-shutdown should be force-shutdown autofs-5.0.7 - fix hesiod check error and use correct $(LIBS) setting autofs-5.0.7 - fix dead LDAP symbolic link when LDAP support is disabled autofs-5.0.7 - add missing libtirpc lib to mount_nfs.so when TIRPC enabled autofs-5.0.7 - use compiler determined by configure instead of hard-coded ones autofs-5.0.7 - remove hard-coded STRIP variable
autofs-5.0.7 - use LIBS for link libraries
autofs-5.0.7 - unbundle NOTSTRIP from DEBUG so they dont depend on each other autofs-5.0.7 - fix occasional build error when enable parallel compiling

Leonardo Chiquitto (1):
autofs-5.0.7 - fix crash due to thread unsafe use of libldap

 CHANGELOG                      |   23 ++++
 Makefile.conf.in               |    3
 Makefile.rules                 |   34 +++--
 aclocal.m4                     |   19 ++-
 autofs.spec                    |    3
 configure                      |  104 +++++++++++++++--
 configure.in                   |    2
 daemon/Makefile                |    2
 daemon/automount.c             |   71 +++++++++--
 daemon/state.c                 |   15 +-
 include/config.h.in            |    3
 include/lookup_ldap.h          |    4 -
 include/macros.h               |    1
 include/master.h               |    1
 lib/Makefile                   |    6 +
 lib/macros.c                   |   28 ++++
 lib/master.c                   |  250 ++++++++++++++++++++++++++++++++++++++--
 man/auto.master.5.in           |    5 +
 man/automount.8                |   23 +++-
 modules/Makefile               |   20 ++-
 modules/cyrus-sasl.c           |   39 +++++-
 modules/lookup_file.c          |   25 +++-
 modules/lookup_ldap.c          |   76 +++++++++++-
 modules/lookup_nisplus.c       |   26 +++-
 modules/lookup_program.c       |   20 +++
 modules/lookup_sss.c           |   22 +++-
 modules/lookup_yp.c            |   23 +++-
 modules/mount_bind.c           |    2
 modules/mount_nfs.c            |   14 ++
 redhat/autofs.sysconfig.in     |    5 +
 samples/autofs.conf.default.in |    5 +
 samples/autofs.service.in      |    2
 32 files changed, 753 insertions(+), 123 deletions(-)
Quoted 2 lines

--
Ian

Quoted 7 lines

HI Ian:
May I suggest you pushing this patch serials to one test branch
(for-next?) in the public repository?
I find this much more convenient for me to test (instead of fetch all the patches and apply myself).
Since this branch is experimental, you can delete it later or rebase to master branch.

Not sure I've done this correctly but try updating and checkout branch pending-temp-v0.

Ian

#31 lore raw
Quoted 91 lines · Ian Kent
Quoted 82 lines · Dennis Lan (dlan)
Quoted 77 lines · Ian Kent

There are patches from several contributors I plan on committing so I thought posting them to the list for final review would be sensible.

I'm not sure when I'll commit the changes but I hope in the next couple of days.

---

Dustin Polke (1):
autofs-5.0.7 - fix compilation of lookup_ldap.c without sasl

Ian Kent (13):
autofs-5.0.7 - don't override LDFLAGS in make rules autofs-5.0.7 - fix a couple of compiler warnings autofs-5.0.7 - add after sssd dependency to unit file autofs-5.0.7 - dont start readmap unless ready autofs-5.0.7 - fix dumpmaps multi output autofs-5.0.7 - try and cleanup after dumpmaps autofs-5.0.7 - teach dumpmaps to output simple key value pairs autofs-5.0.7 - fix syncronize handle_mounts() shutdown autofs-5.0.7 - fix fix wildcard multi map regression autofs-5.0.7 - improve timeout option description autofs-5.0.7 - only probe specific nfs version when requested autofs-5.0.7 - fix bad mkdir permission on create autofs-5.0.7 - setup program map env from macro table

Lan Yixun (dlan) (10):
autofs-5.0.7 - fix compile error with heimdal support enabled autofs-5.0.7 - fix typo forced-shutdown should be force-shutdown autofs-5.0.7 - fix hesiod check error and use correct $(LIBS) setting autofs-5.0.7 - fix dead LDAP symbolic link when LDAP support is disabled autofs-5.0.7 - add missing libtirpc lib to mount_nfs.so when TIRPC enabled autofs-5.0.7 - use compiler determined by configure instead of hard-coded ones autofs-5.0.7 - remove hard-coded STRIP variable
autofs-5.0.7 - use LIBS for link libraries
autofs-5.0.7 - unbundle NOTSTRIP from DEBUG so they dont depend on each other autofs-5.0.7 - fix occasional build error when enable parallel compiling

Leonardo Chiquitto (1):
autofs-5.0.7 - fix crash due to thread unsafe use of libldap

 CHANGELOG                      |   23 ++++
 Makefile.conf.in               |    3
 Makefile.rules                 |   34 +++--
 aclocal.m4                     |   19 ++-
 autofs.spec                    |    3
 configure                      |  104 +++++++++++++++--
 configure.in                   |    2
 daemon/Makefile                |    2
 daemon/automount.c             |   71 +++++++++--
 daemon/state.c                 |   15 +-
 include/config.h.in            |    3
 include/lookup_ldap.h          |    4 -
 include/macros.h               |    1
 include/master.h               |    1
 lib/Makefile                   |    6 +
 lib/macros.c                   |   28 ++++
 lib/master.c                   |  250 ++++++++++++++++++++++++++++++++++++++--
 man/auto.master.5.in           |    5 +
 man/automount.8                |   23 +++-
 modules/Makefile               |   20 ++-
 modules/cyrus-sasl.c           |   39 +++++-
 modules/lookup_file.c          |   25 +++-
 modules/lookup_ldap.c          |   76 +++++++++++-
 modules/lookup_nisplus.c       |   26 +++-
 modules/lookup_program.c       |   20 +++
 modules/lookup_sss.c           |   22 +++-
 modules/lookup_yp.c            |   23 +++-
 modules/mount_bind.c           |    2
 modules/mount_nfs.c            |   14 ++
 redhat/autofs.sysconfig.in     |    5 +
 samples/autofs.conf.default.in |    5 +
 samples/autofs.service.in      |    2
 32 files changed, 753 insertions(+), 123 deletions(-)
Quoted 2 lines

--
Ian

Quoted 7 lines

HI Ian:
May I suggest you pushing this patch serials to one test branch
(for-next?) in the public repository?
I find this much more convenient for me to test (instead of fetch all the patches and apply myself).
Since this branch is experimental, you can delete it later or rebase to master branch.

Quoted 6 lines

Not sure I've done this correctly but try updating and checkout branch pending-temp-v0.

Ian

HI Ian:
what you 've done is correct!
I tested this branch, so far so good..

you can use "git push origin :refs/heads/pending-temp-v0" to delete
this branch in the remote repository
or "git push --force origin HEAD:refs/heads/pending-temp-v0" to override the remote branch with new commit (just in case you need to do this)

Dennis Lan (dlan)

#32 lore raw
In reply to Ian Kent
Quoted 3 lines · Ian Kent

There are patches from several contributors I plan on committing so I thought posting them to the list for final review would be sensible.

I am wondering about my auto.smb patch (see
http://www.spinics.net/lists/autofs/msg00402.html). Back then, you said

Quoted 7 lines

All I'm really saying is that, the program map returns a string that is then used as the map entry, so when it is parsed the variables $UID, $GID, etc. will be present in the macro lookup table as ${UID} (and $UID etc. works) ... so maybe these same variables should be provided in the environment of the program.

Quoted 5 lines

It isn't entirely clear if that is needed since those macro values may be returned in the map entry text with things like echo "/some/string \ ... ${UID} ..." etc. for later translation.

Really, a revised patch depends on this question being answered first.

Nobody has spoken up on this list saying that additional environment variables would need to be set. I am wondering what else needs to be done in order to merge this (IMO very useful) functionality.

Martin

Signature · 13 lines
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering

FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone:			++49 5251 525 2796
Fax:			++49 5251 525 2820
Email:			[email protected]
Internet:		http://ts.fujitsu.com
Company Details:	http://ts.fujitsu.com/imprint
#33 lore raw

I am wondering about my auto.smb patch (see http://www.spinics.net/lists/autofs/msg00402.html).

I can add a comment about that link...

Quoted 2 lines
Quoted 2 lines

OTOH, setting the uid to
the caller is definitely what should be done, IMHO.

That only makes sense if the result is only going to be visible to that one individual.

Linux is a multi-user system, so having automounts that depend on who made the request to do the mount makes no sense if the result is going to be visible to everyone.

I've not seen a system whereby different people have different views of what is mounted.

________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited (registered in England and Wales No. 1047315), which is a member of the GlaxoSmithKline group of companies. The registered address of GlaxoSmithKline Services Unlimited is 980 Great West Road, Brentford, Middlesex TW8 9GS.

#34 lore raw
Quoted 5 lines · Gordon Lack

I am wondering about my auto.smb patch (see http://www.spinics.net/lists/autofs/msg00402.html).

I can add a comment about that link...

Quoted 8 lines
Quoted 2 lines
Quoted 2 lines

OTOH, setting the uid to
the caller is definitely what should be done, IMHO.

That only makes sense if the result is only going to be visible to that one individual.

Linux is a multi-user system, so having automounts that depend on who made the request to do the mount makes no sense if the result is going to be visible to everyone. I've not seen a system whereby different people have different views
of what is mounted.

I'm not sure what you mean. Users can create user-specific CIFS mounts today with "sudo mount -t cifs -o sec=krb5,cruid=$UID,...". The intention of my patch is just to enable autofs to create such mounts.

It would certainly be reasonable to setup autofs such that a separate mount point is used for each user, but that's a configuration issue. In as properly configured environment, the "multiuser" option of mount.cifs can be used to deal with local multiuser/permission issues.

Regards
Martin

P.S.: You are correct that there is an issue with autofs caching the list of mount points if the list of mountable shares returned by a server depends on the credentials provided. AFAICS that can be fixed by configuring the such that all users can see the same shares (but not necessarily access/mount them all).

Quoted 9 lines

________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited (registered in England and Wales No. 1047315), which is a member of the GlaxoSmithKline group of companies. The registered address of GlaxoSmithKline Services Unlimited is 980 Great West Road, Brentford, Middlesex TW8 9GS.

Signature · 13 lines
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering

FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone:			++49 5251 525 2796
Fax:			++49 5251 525 2820
Email:			[email protected]
Internet:		http://ts.fujitsu.com
Company Details:	http://ts.fujitsu.com/imprint
#35 lore raw
Quoted 1 line
Quoted 1 line

I'm not sure what you mean.

Just that I've not seen it, so thanks for the example.

Quoted 2 lines
Quoted 2 lines

Users can create user-specific CIFS mounts today with "sudo mount -t cifs -o sec=krb5,cruid=$UID,...". The intention of my patch is just to enable autofs to create such mounts.

But that leaves the mount permission dependent on who make the first call.

Quoted 4 lines
Quoted 4 lines

P.S.: You are correct that there is an issue with autofs caching the list of mount points if the list of mountable shares returned by a server depends on the credentials provided. AFAICS that can be fixed by configuring the such that all users can see the same shares (but not necessarily access/mount them all).

And once you've done that the UID that needs to be used for each of these mounts is mount-specific, not "who caused the mount"-specific. Which is why I see a problem with it.

________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited (registered in England and Wales No. 1047315), which is a member of the GlaxoSmithKline group of companies. The registered address of GlaxoSmithKline Services Unlimited is 980 Great West Road, Brentford, Middlesex TW8 9GS.

#36 lore raw
Quoted 8 lines · Gordon Lack
Quoted 1 line
Quoted 1 line

I'm not sure what you mean.

Just that I've not seen it, so thanks for the example.

Quoted 2 lines
Quoted 2 lines

Users can create user-specific CIFS mounts today with "sudo mount -t cifs -o sec=krb5,cruid=$UID,...". The intention of my patch is just to enable autofs to create such mounts.

But that leaves the mount permission dependent on who make the first call.

True. But that holds in the manual "mount -t cifs ..." case as well.

Quoted 7 lines
Quoted 4 lines
Quoted 4 lines

P.S.: You are correct that there is an issue with autofs caching the list of mount points if the list of mountable shares returned by a server depends on the credentials provided. AFAICS that can be fixed by configuring the such that all users can see the same shares (but not necessarily access/mount them all).

And once you've done that the UID that needs to be used for each of these mounts is mount-specific, not "who caused the mount"-specific. Which is why I see a problem with it.

Do you have security concerns, or is it just that you don't consider it useful? I can just say that I find it very useful working on a Linux workstation in an AD-dominated environment. It's much more practical than using any of the "Network Browsing" utilities provided by GNOME and the like. Just my personal opinion.

I admit I have been using it mostly on my workstation, where I am the only user.

Martin

Quoted 11 lines

________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited (registered in England and Wales No. 1047315), which is a member of the GlaxoSmithKline group of companies. The registered address of GlaxoSmithKline Services Unlimited is 980 Great West Road, Brentford, Middlesex TW8 9GS.

Signature · 13 lines
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering

FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone:			++49 5251 525 2796
Fax:			++49 5251 525 2820
Email:			[email protected]
Internet:		http://ts.fujitsu.com
Company Details:	http://ts.fujitsu.com/imprint
#37 lore raw
Quoted 3 lines
Quoted 1 line

But that leaves the mount permission dependent on who make the first call.

True. But that holds in the manual "mount -t cifs ..." case as well.

The manual mount will be done by a specific individual who (hopefully) knows what they are doing.

An automount can be done by a non-determined account and so have a non-determinate outcome once you put per-caller variables into the rule.

Quoted 4 lines
Quoted 2 lines

And once you've done that the UID that needs to be used for each of
these mounts is mount-specific, not "who caused the mount"-specific. Which is why I see a problem with it.

Do you have security concerns, or is it just that you don't consider it useful?

Both.
Its presence would encourage its use.
You will then find users who set it up for themselves and then get confused when another account has made the mount and access is wrong, but everything "looks" correct. Mind you - the security issue is about using cifs mounts on a system which has the potential for >1 concurrent user anyway.

Quoted 1 line

But I admit I have been using it mostly on my workstation, where I am the only user.

So you can set-up the rules so that they contain just your id. And make sure you lock-out all other users once you have anything mounted.

________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited (registered in England and Wales No. 1047315), which is a member of the GlaxoSmithKline group of companies. The registered address of GlaxoSmithKline Services Unlimited is 980 Great West Road, Brentford, Middlesex TW8 9GS.

#38 lore raw
Quoted 7 lines · Gordon Lack
Quoted 3 lines
Quoted 1 line

But that leaves the mount permission dependent on who make the first call.

True. But that holds in the manual "mount -t cifs ..." case as well.

The manual mount will be done by a specific individual who (hopefully) knows what they are doing.

An automount can be done by a non-determined account and so have a non-determinate outcome once you put per-caller variables into the rule.

What my patch does is to run the map script under the UID of the user requesting the mount, rather than root. That is actually an improvement of the security situation, AFAICS.

Quoted 9 lines
Quoted 4 lines
Quoted 2 lines

And once you've done that the UID that needs to be used for each of
these mounts is mount-specific, not "who caused the mount"-specific. Which is why I see a problem with it.

Do you have security concerns, or is it just that you don't consider it useful?

Both.
Its presence would encourage its use.
You will then find users who set it up for themselves and then get confused when another account has made the mount and access is wrong, but everything "looks" correct. Mind you - the security issue is about using cifs mounts on a system which has the potential for >1 concurrent user anyway.

Please check the "multiuser" option of mount.cifs ("With this option, the client ... creates a new session with the server using the user's credentials whenever a new user accesses the mount"). With that option, I see no major difference between CIFS and NFS automounts, security-wise. IMO combining autofs and "multiuser" is exactly the desired behavior in an AD environment. It lets the server decide access rights based on the credentials provided.

Quoted 4 lines
Quoted 1 line

But I admit I have been using it mostly on my workstation, where I am the only user.

So you can set-up the rules so that they contain just your id. And make sure you lock-out all other users once you have anything mounted.

I don't think that's necessary, see above.

Your arguments would apply equally well to users clicking on "Network Environment" or similar in their GUI. It's a (sad) fact in life that many of us have to work in Windows-dominated IT environments.

Martin

Signature · 13 lines
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering

FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone:			++49 5251 525 2796
Fax:			++49 5251 525 2820
Email:			[email protected]
Internet:		http://ts.fujitsu.com
Company Details:	http://ts.fujitsu.com/imprint
#39 lore raw
Quoted 2 lines
Quoted 2 lines

What my patch does is to run the map script under the UID of the user requesting the mount, rather than root. That is actually an improvement of the security situation, AFAICS.

Possibly not.

Quoted 2 lines
Quoted 2 lines

Please check the "multiuser" option of mount.cifs ("With this option, the client ... creates a new session with the server using the user's credentials whenever a new user accesses the mount").

It also says:

Furthermore, when unix extensions aren't in use and the administrator has not overriden ownership using the uid= or gid= options, ownership of files is presented as the current user accessing the share.

Which I take to mean that if you are specifying $UID in the mount options then you've just foiled this bit, the bit that you actually want.

Unless cruid is different? (Never used it - or Kerberos - so don't know).

Quoted 3 lines
Quoted 3 lines

With that option, I see no major difference between CIFS and NFS automounts, security-wise. IMO combining autofs and "multiuser" is exactly the desired behavior in an AD environment. It lets the server decide access rights based on the credentials provided.

You could well be right. I was just raising it as something that may lead to security issues in general.

Quoted 1 line
Quoted 1 line

It's a (sad) fact in life that many of us have to work in Windows-dominated IT environments.

True. Although putting everything on network file servers which can farm out the same files using both cifs and nfs can simplify the clients.

________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited (registered in England and Wales No. 1047315), which is a member of the GlaxoSmithKline group of companies. The registered address of GlaxoSmithKline Services Unlimited is 980 Great West Road, Brentford, Middlesex TW8 9GS.

#40 lore raw
Quoted 4 lines · Gordon Lack
Quoted 2 lines
Quoted 2 lines

What my patch does is to run the map script under the UID of the user requesting the mount, rather than root. That is actually an improvement of the security situation, AFAICS.

Possibly not.

Can you explain?

Quoted 11 lines
Quoted 2 lines
Quoted 2 lines

Please check the "multiuser" option of mount.cifs ("With this option, the client ... creates a new session with the server using the user's credentials whenever a new user accesses the mount").

It also says:

Furthermore, when unix extensions aren't in use and the administrator has not overriden ownership using the uid= or gid= options, ownership of files is presented as the current user accessing the share.

Which I take to mean that if you are specifying $UID in the mount options then you've just foiled this bit, the bit that you actually want.

Permission checks are done on the server.

On the client, without unix extensions, the user/group IDs of files may be displayed wrongly. That may confuse users because they may not be able to open files listed as owned by themselves, but it's not a security problem.

Martin

Signature · 13 lines
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering

FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone:			++49 5251 525 2796
Fax:			++49 5251 525 2820
Email:			[email protected]
Internet:		http://ts.fujitsu.com
Company Details:	http://ts.fujitsu.com/imprint
#41 lore raw
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines

What my patch does is to run the map script under the UID of the user requesting the mount, rather than root. That is actually an improvement of the security situation, AFAICS.

Possibly not.

Can you explain?

It won't be an improvement if it enables something that is inherently insecure.

Quoted 3 lines
Quoted 1 line

Which I take to mean that if you are specifying $UID in the mount options then you've just foiled this bit, the bit that you actually want.

Permission checks are done on the server.

Then why do you need to specify a specific UID at mount time?

Quoted 3 lines

On the client, without unix extensions, the user/group IDs of files may be displayed wrongly. That may confuse users because they may not be able to open files listed as owned by themselves, but it's not a security problem.

OK - so now I'm confused. you mentioned the multiuser option to mount.cifs as something that enables individual users to get their own individual access permissions. But you're also saying (above) that they won't.

It's this lack of specificity that worries me.

If we add this code
it will enable this feature
and remove this current anomaly/mis-feature
which will provide this end-user enhancement.

________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited (registered in England and Wales No. 1047315), which is a member of the GlaxoSmithKline group of companies. The registered address of GlaxoSmithKline Services Unlimited is 980 Great West Road, Brentford, Middlesex TW8 9GS.

#42 lore raw
Quoted 8 lines · Gordon Lack
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines

What my patch does is to run the map script under the UID of the user requesting the mount, rather than root. That is actually an improvement of the security situation, AFAICS.

Possibly not.

Can you explain?

It won't be an improvement if it enables something that is inherently insecure.

I don't think that it's inherently insecure, at least not any more than clicking on a network share in nautilus et al.

UID/GID mapping between client and server and permission control is a general security challenge for Network file systems. That's not specific to CIFS, NFS has similar problems. Yet aufofs has had the "-hosts" map for a long time. My patch aims at something similar to '-hosts' for CIFS, using kerberos credentials for access control. For both NFS and CIFS, this kind of feature must not be enabled without proper security considerations, but that shouldn't be used as an argument not to implement the feature at all.

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Which I take to mean that if you are specifying $UID in the mount options then you've just foiled this bit, the bit that you actually want.

Permission checks are done on the server.

Then why do you need to specify a specific UID at mount time?

Depending on configuration, CIFS won't let you mount anything without credentials. You may have to be authorized just to see the list of shares.

Once a user on a Windows client is logged in in AD (Kerberos), he will usually see all relevant shares and be able to access them in various ways. My goal is to provide a similarly easy access to users with AD credentials on Linux clients, and one that works on the command line too, not only from the GUI.

Quoted 6 lines
Quoted 3 lines

On the client, without unix extensions, the user/group IDs of files may be displayed wrongly. That may confuse users because they may not be able to open files listed as owned by themselves, but it's not a security problem.

OK - so now I'm confused. you mentioned the multiuser option to mount.cifs as something that enables individual users to get their own individual access permissions. But you're also saying (above) that they won't.

They will have individual access permissions. But without CIFS Unix extensions, stat(2) will not return real Unix user and group IDs on files or directories; instead every user will see his own user and group ID. The real user and group IDs on the server are the Active Directory Ids of the user owning the file.

Regards
Martin

Signature · 13 lines
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering

FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone:			++49 5251 525 2796
Fax:			++49 5251 525 2820
Email:			[email protected]
Internet:		http://ts.fujitsu.com
Company Details:	http://ts.fujitsu.com/imprint
#43 lore raw
Quoted 2 lines
Quoted 2 lines

UID/GID mapping between client and server and permission control is a general security challenge for Network file systems. That's not specific to CIFS, NFS has similar problems.

Global identities. Don't set up Samba (etc.) to create local ones.

Quoted 1 line
Quoted 1 line

Yet aufofs has had the "-hosts" map for a long time.

Should _only_ ever be used on admin hosts for admin tasks as it hard-wires physical locations into pathnames (one of the things which automount is able to remove).

Quoted 2 lines
Quoted 2 lines

Depending on configuration, CIFS won't let you mount anything without credentials. You may have to be authorized just to see the list of shares.

So are you saying that the example you were giving, to use multiuser on a cifs mount, was so that the mount could take place under the user's Kerberos credentials (which might work) rather than root's (who might not have them)? But that once the mount was in place each user's access would be under their own credentials (assuming they have them)?

In which case I'd agree that this isn't a security issue.

________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited (registered in England and Wales No. 1047315), which is a member of the GlaxoSmithKline group of companies. The registered address of GlaxoSmithKline Services Unlimited is 980 Great West Road, Brentford, Middlesex TW8 9GS.

#44 lore raw
Quoted 8 lines · Gordon Lack
Quoted 2 lines
Quoted 2 lines

Depending on configuration, CIFS won't let you mount anything without credentials. You may have to be authorized just to see the list of shares.

So are you saying that the example you were giving, to use multiuser on a cifs mount, was so that the mount could take place under the user's Kerberos credentials (which might work) rather than root's (who might not have them)? But that once the mount was in place each user's access would be under their own credentials (assuming they have them)?

In which case I'd agree that this isn't a security issue.

This is my understandning of how "multiuser" works, yes.

Martin

Quoted 10 lines

________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited (registered in England and Wales No. 1047315), which is a member of the GlaxoSmithKline group of companies. The registered address of GlaxoSmithKline Services Unlimited is 980 Great West Road, Brentford, Middlesex TW8 9GS.

Signature · 13 lines
Dr. Martin Wilck
PRIMERGY System Software Engineer
x86 Server Engineering

FUJITSU
Fujitsu Technology Solutions GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn, Germany
Phone:			++49 5251 525 2796
Fax:			++49 5251 525 2820
Email:			[email protected]
Internet:		http://ts.fujitsu.com
Company Details:	http://ts.fujitsu.com/imprint
#45 lore raw
Quoted 3 lines
Quoted 1 line

In which case I'd agree that this isn't a security issue.

This is my understandning of how "multiuser" works, yes.

OK.

I forsee some testing being done on my little ARM/MIPS systems at home...just don't wait for results....

________________________________

This e-mail was sent by GlaxoSmithKline Services Unlimited (registered in England and Wales No. 1047315), which is a member of the GlaxoSmithKline group of companies. The registered address of GlaxoSmithKline Services Unlimited is 980 Great West Road, Brentford, Middlesex TW8 9GS.

author maintainer
#46 lore raw
In reply to Martin Wilck
Quoted 19 lines · Martin Wilck
Quoted 3 lines · Ian Kent

There are patches from several contributors I plan on committing so I thought posting them to the list for final review would be sensible.

I am wondering about my auto.smb patch (see
http://www.spinics.net/lists/autofs/msg00402.html). Back then, you said

Quoted 11 lines

All I'm really saying is that, the program map returns a string that is then used as the map entry, so when it is parsed the variables $UID, $GID, etc. will be present in the macro lookup table as ${UID} (and $UID etc. works) ... so maybe these same variables should be provided in the environment of the program.

It isn't entirely clear if that is needed since those macro values may be returned in the map entry text with things like echo "/some/string \ ... ${UID} ..." etc. for later translation.

Quoted 5 lines
Quoted 1 line

Really, a revised patch depends on this question being answered first.

Nobody has spoken up on this list saying that additional environment variables would need to be set. I am wondering what else needs to be done in order to merge this (IMO very useful) functionality.

I like the idea of the setuid() but it's probably more convenient for me to set the standard macro variables (that are set when the entry is parsed) in the environment before running the program map. Also setuid() changes what some may expect to be the case so that's probably not a good idea.

One of the current patches in the queue sets existing macro variables in the environment, in order to pass parameters to a program map, so all that would need to be done is a call to a function that adds the standard variables to the macro table for that to happen.

I posted a question about the second patch and will wait for the answer to that.

Quoted 3 lines

Martin