Package: libpam-mount
Followup-For: Bug #302024

I just hit this bug, and have identified the problem.

The documentation suggests to me too that @common-pammount should be put
twice in the relevant /etc/pam.d/ file.

However, libpam-mount's pam_mount.c assumes at pam_sm_session_end-time,
it's safe to free the configuration. This is not true, as
pam_sm_session_end is called twice under this configuration.

Instead, I've patched it to add the config_t as a pam data item, only
once, with a cleanup function that calls freeconfig(), which will only
be done once, when the application calls pam_end. It has to be done this
way because if you add a new data item with the same name, the old one's
cleanup function is called (which seems contrary to the documentation on
opengroup.org...) but happily readconfig() is only called in once place,
so it's easy to put a protected set_data there.

This is one more step on the way to elinimating the global config_t in
favour of the PAM data storage, which it looks like the upstream author
is working towards.

With this patch, session usage counting works well. XDM is still broken
however, I'm going to try and track that down this week.

valgrind does not complain about anything in libpam-mount anymore.

The second stanza of the patch fixes a different error valgrind noticed,
which is that sa_mask is not being initialised for the call to
sigaction.

The final stanza in fmt_ptrn.c is removing an apparently unneccesary
cast. I can't remeber what prompted me to do this, but it was something
from valgrind.

--- libpam-mount-0.9.25.orig/src/pam_mount.c
+++ libpam-mount-0.9.25/src/pam_mount.c
@@ -68,6 +68,16 @@
        }
 }
 
+/* ============================ clean_global_config () ==================== */
+/* INPUT: pamh; data; errcode
+ * SIDE AFFECTS: config_t variable 'data' is free'd.
+ * NOTE: this is registered as a PAM callback function and called directly */
+void clean_config(pam_handle_t * pamh, void *data, int errcode)
+{
+       w4rn("pam_mount: clean global config (%d)\n", errcode);
+       freeconfig(*(config_t *)data);
+}
+
 /* ============================ clean_system_authtok () ==================== */
 /* INPUT: pamh; data; errcode
  * SIDE AFFECTS: if data does not point to NULL then it is zeroized and freed 
@@ -252,10 +262,13 @@
        char *_argv[MAX_PAR + 1];
        pid_t pid;
        struct sigaction sact, oldsact;
+       sigset_t sigmask;
 
        /* avoid bomb on command exiting before count read */
        sact.sa_handler = SIG_DFL;
        sact.sa_flags = 0;
+       sigemptyset(&sigmask);
+       sact.sa_mask = sigmask;
        if (sigaction(SIGPIPE, &sact, &oldsact) < 0) {
                fnval = -1;
                goto _nosigact_return;
@@ -313,6 +326,8 @@
        int ret = PAM_SUCCESS;
        char *system_authtok;
        const char *pam_user = NULL;
+       void *tmp;
+       int getval;
 
        assert(pamh);
 
@@ -352,6 +367,15 @@
                ret = PAM_SERVICE_ERR;
                goto _return;
        }
+       if ((getval = pam_get_data(pamh, "pam_mount_config", &tmp))
+               == PAM_NO_MODULE_DATA)
+               if ((ret =
+                        pam_set_data(pamh, "pam_mount_config", &config,
+                                 clean_config)) != PAM_SUCCESS) {
+                       l0g("pam_mount: %s\n",
+                               "error trying to save config structure");
+                       goto _return;
+               }
        w4rn("pam_mount: %s\n", "back from global readconfig");
        if (!strlen(config.luserconf))
                w4rn("pam_mount: %s\n",
@@ -467,7 +491,7 @@
                w4rn("pam_mount: %s seems to have other remaining open 
sessions\n", config.user);
 /* end root priv. */
       _return:
-       freeconfig(config);
+//     freeconfig(config);
        w4rn("pam_mount: pam_mount execution complete\n");
        return ret;
 }
only in patch2:
unchanged:
--- libpam-mount-0.9.25.orig/src/fmt_ptrn.c
+++ libpam-mount-0.9.25/src/fmt_ptrn.c
@@ -604,7 +604,7 @@
        assert(_fmt_ptrn_t_valid(x));
        assert(p != NULL);
 
-       pattern = orig_ptr = g_strdup((char *) p); /* stupid cast */
+       pattern = orig_ptr = g_strdup(p);
        while (*pattern != 0x00) {
                if (*pattern == '%' && *(pattern + 1) == '%') {
                        /* Handle %%(...), which should be filled as %(...). */


-- System Information:
Debian Release: testing/unstable
  APT prefers unstable
  APT policy: (990, 'unstable'), (950, 'unstable'), (900, 'experimental')
Architecture: i386 (i686)
Shell:  /bin/sh linked to /bin/bash
Kernel: Linux 2.6.12
Locale: LANG=en_AU.UTF-8, LC_CTYPE=en_AU.UTF-8 (charmap=UTF-8)

-- 
Paul "TBBle" Hampson, [EMAIL PROTECTED]
8th year CompSci/Asian Studies student, ANU

Shorter .sig for a more eco-friendly paperless office.

Attachment: pgpkHSVLrYZFM.pgp
Description: PGP signature

Reply via email to