On Thu, 2012-10-11 at 10:00 +0200, Jakub Hrozek wrote: > From 98c8a6b92db2872083473b4ce0761bffc919e847 Mon Sep 17 00:00:00 2001 > From: Jakub Hrozek <jhro...@redhat.com> > Date: Thu, 4 Oct 2012 19:08:08 +0200 > Subject: [PATCH] PAM: close socket fd with pam_set_data > > https://fedorahosted.org/sssd/ticket/1569 > --- > src/sss_client/common.c | 6 ++++++ > src/sss_client/pam_sss.c | 26 ++++++++++++++++++++++++++ > src/sss_client/sss_cli.h | 2 ++ > 3 files changed, 34 insertions(+) > > diff --git a/src/sss_client/common.c b/src/sss_client/common.c > index > 1ef3ba15e5a86952a05de29c9db212fc829111bc..a4d523cdf45665c2e1b4984cc9b6db14c3d05340 > 100644 > --- a/src/sss_client/common.c > +++ b/src/sss_client/common.c > @@ -794,6 +794,12 @@ errno_t check_server_cred(int sockfd) > #endif > return 0; > } > + > +int *sss_pam_get_socket(void) > +{ > + return &sss_cli_sd; > +} > + > int sss_pam_make_request(enum sss_cli_command cmd, > struct sss_cli_req_data *rd, > uint8_t **repbuf, size_t *replen, > diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c > index > efbc48b6ef0458bb79dad53f81c584611f2be22d..92c90742e1931cf063a769a13befa8ec3567969a > 100644 > --- a/src/sss_client/pam_sss.c > +++ b/src/sss_client/pam_sss.c > @@ -54,6 +54,7 @@ > #define FLAGS_USE_AUTHTOK (1 << 2) > > #define PWEXP_FLAG "pam_sss:password_expired_flag" > +#define FD_DESTRUCTOR "pam_sss:fd_destructor" > > #define PW_RESET_MSG_FILENAME_TEMPLATE SSSD_CONF_DIR"/customize/% > s/pam_sss_pw_reset_message.%s" > #define PW_RESET_MSG_MAX_SIZE 4096 > @@ -122,6 +123,24 @@ static void free_exp_data(pam_handle_t *pamh, > void *ptr, int err) > ptr = NULL; > } > > +static void close_fd(pam_handle_t *pamh, void *ptr, int err) > +{ > + int fd = *((int *) ptr); > + > + if (err & PAM_DATA_REPLACE) { > + /* Nothing to do */ > + return; > + } > + > + if (fd == -1) { > + /* fd not yet initialized */ > + return; > + } > + > + D(("Closing the fd")); > + close(fd); > +}
NACK, ^^^^^^^^^ you are closing the fd and leaving sss_cli_sd untouched, which means it will still point to a random file descriptor. Also I do not understand why this whole dance in passing pointers and the sss_pam_get_socket() function, seem all very unnecessary. . sss_cli_sd is a global variable which means: a) you can access it directly b) you need mutexes to access it (see sss_pam_make_request()). Simo. -- Simo Sorce * Red Hat, Inc * New York _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
