PAM response memory deallocation question
Gary Flynn
I've been trying to learn my way around PAM programming and
have a working conversation module except I think it has
a memory leak :(
I understand that the PAM module should release memory
associated with the pointer to the response structure.
Does it also release memory associated with the pointer to
the character pointer structure elements inside the response
structure?
thanks,
gary
Paul D. Boyle
: > Does it also release memory associated with the pointer to
: > the character pointer structure elements inside the response
: > structure?
Frank Cusack (fcu...@iconnet.net) wrote:
: yes.
I don't think so. If you read the FAQ for the comp.lang.c newsgroup,
question 7.23 points out that malloc() and free() do not know anything
about the pointers in a struct which point to other allocated memory.
The rule of thumb is that for each call to malloc() you should have a
corresponding call to free().
Paul
--
Paul D. Boyle | bo...@laue.chem.ncsu.edu
Director, X-ray Structural Facility | phone: (919) 515-7362
Department of Chemistry - Box 8204 | FAX: (919) 515-5079
North Carolina State University |
Raleigh, NC, 27695-8204
http://laue.chem.ncsu.edu/web/xray.welcome.html
Frank Cusack
> : Gary Flynn <fly...@jmu.edu> writes:
> : > Does it also release memory associated with the pointer to
> : > the character pointer structure elements inside the response
> : > structure?
>
> Frank Cusack (fcu...@iconnet.net) wrote:
> : yes.
>
> I don't think so. If you read the FAQ for the comp.lang.c newsgroup,
> question 7.23 points out that malloc() and free() do not know anything
> about the pointers in a struct which point to other allocated memory.
> The rule of thumb is that for each call to malloc() you should have a
> corresponding call to free().
>
> Paul
>
Right, but the PAM code is responsible for free()ing the char *'s
before free()ing the resp *.
~frank
--
* I am Pentium of Borg. Division is futile. You will be approximated. *
* PGP ID: C001AA75 -|- fcu...@iconnet.net *
Paul D. Boyle
: bo...@laue.chem.ncsu.edu (Paul D. Boyle) writes:
: > I don't think so. If you read the FAQ for the comp.lang.c newsgroup,
: > question 7.23 points out that malloc() and free() do not know anything
: > about the pointers in a struct which point to other allocated memory.
: > The rule of thumb is that for each call to malloc() you should have a
: > corresponding call to free().
: >
: > Paul
: >
: Right, but the PAM code is responsible for free()ing the char *'s
: before free()ing the resp *.
Thanks for the clarification.
Gary Flynn
code below to see what I'm doing wrong. This is basically the
check_user application in the Linux PAM Application Programmers
Guide. I wrote the conversation routine. It just authenticates
over and over again with a fixed username and password.
However, my code apparantly has a memory leak as the SIZE
parameter of the ps command continues to grow. I've tested
against two different PAM modules on RedHat 5.2. I've tested
against the standard system login PAM modules on both Redhat 5.2
and HPUX 11.0. It authenticates fine but I can't seem to get rid
of that memory leak.
What bonehead thing am I doing wrong?
thanks,
gary
This has been stripped to the bare essentials.
#define USERNAME "username"
#define PASSWORD "password"
#include <security/pam_appl.h>
#include <string.h>
#include <malloc.h>
#include <stdio.h>
int test_conv(int num_msg, const struct pam_message **msg,
struct pam_response **resp, void *appdata_ptr)
{
//Allocate memory for PAM response structure
*resp=(struct pam_response *)malloc(PAM_MAX_RESP_SIZE);
if( !*resp )
exit(1);
//Allocate memory for char* element in PAM response structure
//I hope like hell PAM knows how to deallocate this :)
(*resp)->resp=(char *)malloc(20);
if( !((*resp)->resp) )
exit(1);
//Move the password into the response
strncpy( (*resp)->resp, PASSWORD, 15 );
//Set the return code in the response
(*resp)->resp_retcode=0;
return PAM_SUCCESS;
}
static struct pam_conv conv = {
test_conv,
NULL
};
int main( int argc, char *argv[])
{
pam_handle_t *pamh = NULL;
int retval;
const char *user=USERNAME;
// Authenticate repeatedly
while(1){
retval = pam_start("check_user", user, &conv, &pamh);
if (retval != PAM_SUCCESS){
printf("pam_start returned with %d\n", retval);
exit(1);
}
retval = pam_authenticate(pamh, 0);
if (retval != PAM_SUCCESS){
printf("pam_authenticate returned with %d\n", retval);
exit(1);
}
if (pam_end(pamh, retval) != PAM_SUCCESS){
pamh = NULL;
fprintf(stderr, "check_user: failed to release PAM resources\n");
exit(1);
}
} //end while(1)
return ( retval == PAM_SUCCESS ? 0:1 );
}
Frank Cusack
> I wonder if someone could look at the PAM conversation function
> code below to see what I'm doing wrong. This is basically the
> check_user application in the Linux PAM Application Programmers
> Guide. I wrote the conversation routine. It just authenticates
> over and over again with a fixed username and password.
>
> However, my code apparantly has a memory leak as the SIZE
> parameter of the ps command continues to grow. I've tested
> against two different PAM modules on RedHat 5.2. I've tested
> against the standard system login PAM modules on both Redhat 5.2
> and HPUX 11.0. It authenticates fine but I can't seem to get rid
> of that memory leak.
>
> What bonehead thing am I doing wrong?
Well, you ignore the num_msg parameter. And `//' is not a comment
delimiter in "C". Other than that it looks good.
Note that the Linux pam_unix module has this code:
int _set_auth_tok( pam_handle_t *pamh,
int flags, int argc,
const char **argv )
{
/* ... */
if ( ( retval = converse( pamh, 1 , pmsg, &resp ) ) != PAM_SUCCESS )
return retval;
if ( resp )
{
if ( ( flags & PAM_DISALLOW_NULL_AUTHTOK ) &&
resp[0].resp == NULL )
{
free( resp );
return PAM_AUTH_ERR;
}
p = resp[ 0 ].resp;
/* This could be a memory leak. If resp[0].resp
is malloc()ed, then it has to be free()ed!
-- alex
*/
resp[ 0 ].resp = NULL;
}
/* ... */
}
so it is a known leak.
Also note that the Solaris man page for pam_start says:
The structure pam_response is used by the authentication
service to get the user's response back from the application
or user. The storage used by pam_response has to be allo-
cated by the application and freed by the PAM modules.
It does not say that the pam_response * has to be freed by
the modules, it says "the storage used" which I read as
including the char * in the struct pam_response.
The linux PAM docos say specifically that the struct must be
freed by the module, and say nothing about the char * within it.
It's obvious that the module should free the char *, at least to me.
My kerberos 5 module does free the char * within the pam_response
struct, with no ill effect on any applications (which I expect
would have problems if freeing the same memory twice).
Your test application does leak memory on Solaris also, so
this is probably a bug in all implementations.
However, the PAM design takes for granted that the API is used
by applications that fork/exec for each new authentication, so
this type of memory loss is (fairly) inconsequential.
~frank
[test program elided]
Gary Flynn
Frank Cusack wrote:
>
> Well, you ignore the num_msg parameter. And `//' is not a comment
> delimiter in "C". Other than that it looks good.
My full program takes into account num_msg and types of PAM
messages. I just wanted to send the simplified code.
// works good on my compiler. New habits are hard
to break. I've been working with C++ for the past 9
months and had to jump back into C :)
> Note that the Linux pam_unix module has this code:
>
> p = resp[ 0 ].resp;
>
> /* This could be a memory leak. If resp[0].resp
> is malloc()ed, then it has to be free()ed!
> -- alex
> */
>
> resp[ 0 ].resp = NULL;
Kind of makes you wonder why they didn't free it?
>
> Your test application does leak memory on Solaris also, so
> this is probably a bug in all implementations.
>
> However, the PAM design takes for granted that the API is used
> by applications that fork/exec for each new authentication, so
> this type of memory loss is (fairly) inconsequential.
Thanks Frank. That makes me feel better. I'll have to sit and
think how that is going to affect my use of PAM, though. I'm
not a programmer so I was *sure* I was doing something stupid :)
gary