[SSSD] [PATCH] PAM: close socket fd with pam_set_data
Simo Sorce
simo at redhat.com
Thu Oct 11 19:21:07 UTC 2012
On Thu, 2012-10-11 at 20:23 +0200, Jakub Hrozek wrote:
> On Thu, Oct 11, 2012 at 09:30:29AM -0400, Simo Sorce wrote:
> > On Thu, 2012-10-11 at 10:00 +0200, Jakub Hrozek wrote:
> > > From 98c8a6b92db2872083473b4ce0761bffc919e847 Mon Sep 17 00:00:00 2001
> > > From: Jakub Hrozek <jhrozek at 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, ^^^^^^^^^
>
> I'm sorry Simo but your Nack came too late after I pushed the patch and
> before I sent a pushmail, so I'll attach a fix.
>
> As discussed off-list already I will CC you with any future client
> patches.
>
> > you are closing the fd and leaving sss_cli_sd untouched, which means it
> > will still point to a random file descriptor.
> >
>
> fixed
>
> > 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
>
> Yes and no, it's local to the sss_client/common.c module, so I moved
> closing the socket there.
>
> > b) you need mutexes to access it (see sss_pam_make_request()).
>
> fixed
>
> The attached fix should amend the code as you suggested.
ACK
Simo.
--
Simo Sorce * Red Hat, Inc * New York
More information about the sssd-devel
mailing list