OpenPAM/OpenPAM
OpenPAM/OpenPAM
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Newer versions of clang take __nonnull__ annotations into account not only

Browse source 
when compiling code that calls the function, but also when compiling the
function itself.  This means that NULL checks in the function trigger
condition-always-false warnings.  We have a choice between disabling these
warnings, removing the __nonnull__ annotations, or removing the NULL checks.
We prefer to keep the annotations and warnings and remove the checks.  In
all cases, passing NULL to the function in question will result in a
segmentation fault, which is often easier to debug than an error return,
especially when most of these checks were for the PAM handle, which can only
be NULL if the caller ignored an error return from pam_start().


git-svn-id: svn+ssh://svn.openpam.org/svn/openpam/trunk@913 185d5e19-27fe-0310-9dcf-9bff6b9f3609
This commit is contained in:
Dag-Erling Smørgrav 2017-01-21 15:11:12 +00:00
parent a18c87672e
commit e936857588
16 changed files with 37 additions and 80 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
include/security/pam_appl.h
View file

@ -72,7 +72,8 @@ pam_close_session(pam_handle_t *_pamh,
int
pam_end(pam_handle_t *_pamh,
int _status);
int _status)
OPENPAM_NONNULL((1));
int
pam_get_data(const pam_handle_t *_pamh,

2
lib/libpam/openpam_dispatch.c
View file

@ -69,8 +69,6 @@ openpam_dispatch(pam_handle_t *pamh,
int debug;
ENTER();
if (pamh == NULL)
RETURNC(PAM_SYSTEM_ERR);
/* prevent recursion */
if (pamh->current != NULL) {

2
lib/libpam/openpam_findenv.c
View file

@ -59,8 +59,6 @@ openpam_findenv(pam_handle_t *pamh,
int i;
ENTER();
if (pamh == NULL)
RETURNN(-1);
for (i = 0; i < pamh->env_count; ++i)
if (strncmp(pamh->env[i], name, len) == 0 &&
pamh->env[i][len] == '=')

27
lib/libpam/openpam_impl.h
View file

@ -130,19 +130,28 @@ struct pam_handle {
/*
* Internal functions
*/
int openpam_configure(pam_handle_t *, const char *);
int openpam_dispatch(pam_handle_t *, int, int);
int openpam_findenv(pam_handle_t *, const char *, size_t);
pam_module_t *openpam_load_module(const char *);
void openpam_clear_chains(pam_chain_t **);
int openpam_configure(pam_handle_t *, const char *)
OPENPAM_NONNULL((1));
int openpam_dispatch(pam_handle_t *, int, int)
OPENPAM_NONNULL((1));
int openpam_findenv(pam_handle_t *, const char *, size_t)
OPENPAM_NONNULL((1,2));
pam_module_t *openpam_load_module(const char *)
OPENPAM_NONNULL((1));
void openpam_clear_chains(pam_chain_t **)
OPENPAM_NONNULL((1));
int openpam_check_desc_owner_perms(const char *, int);
int openpam_check_path_owner_perms(const char *);
int openpam_check_desc_owner_perms(const char *, int)
OPENPAM_NONNULL((1));
int openpam_check_path_owner_perms(const char *)
OPENPAM_NONNULL((1));
#ifdef OPENPAM_STATIC_MODULES
pam_module_t *openpam_static(const char *);
pam_module_t *openpam_static(const char *)
OPENPAM_NONNULL((1));
#endif
pam_module_t *openpam_dynamic(const char *);
pam_module_t *openpam_dynamic(const char *)
OPENPAM_NONNULL((1));
#define FREE(p) \
do { \

8
lib/libpam/pam_end.c
View file

@ -60,8 +60,6 @@ pam_end(pam_handle_t *pamh,
int i;
ENTER();
if (pamh == NULL)
RETURNC(PAM_SYSTEM_ERR);
/* clear module data */
while ((dp = pamh->module_data) != NULL) {
@ -91,12 +89,6 @@ pam_end(pam_handle_t *pamh,
RETURNC(PAM_SUCCESS);
}
/*
* Error codes:
*
* PAM_SYSTEM_ERR
*/
/**
* The =pam_end function terminates a PAM transaction and destroys the
* corresponding PAM context, releasing all resources allocated to it.

2
lib/libpam/pam_get_authtok.c
View file

@ -76,8 +76,6 @@ pam_get_authtok(pam_handle_t *pamh,
int pitem, r, style, twice;
ENTER();
if (pamh == NULL || authtok == NULL)
RETURNC(PAM_SYSTEM_ERR);
*authtok = NULL;
twice = 0;
switch (item) {

3
lib/libpam/pam_get_data.c
View file

@ -60,8 +60,6 @@ pam_get_data(const pam_handle_t *pamh,
pam_data_t *dp;
ENTERS(module_data_name);
if (pamh == NULL)
RETURNC(PAM_SYSTEM_ERR);
for (dp = pamh->module_data; dp != NULL; dp = dp->next) {
if (strcmp(dp->name, module_data_name) == 0) {
*data = (void *)dp->data;
@ -74,7 +72,6 @@ pam_get_data(const pam_handle_t *pamh,
/*
* Error codes:
*
* PAM_SYSTEM_ERR
* PAM_NO_MODULE_DATA
*/

3
lib/libpam/pam_get_item.c
View file

@ -59,8 +59,6 @@ pam_get_item(const pam_handle_t *pamh,
{
ENTERI(item_type);
if (pamh == NULL)
RETURNC(PAM_SYSTEM_ERR);
switch (item_type) {
case PAM_SERVICE:
case PAM_USER:
@ -86,7 +84,6 @@ pam_get_item(const pam_handle_t *pamh,
* Error codes:
*
* PAM_SYMBOL_ERR
* PAM_SYSTEM_ERR
*/
/**

2
lib/libpam/pam_get_user.c
View file

@ -69,8 +69,6 @@ pam_get_user(pam_handle_t *pamh,
int r;
ENTER();
if (pamh == NULL || user == NULL)
RETURNC(PAM_SYSTEM_ERR);
r = pam_get_item(pamh, PAM_USER, (const void **)user);
if (r == PAM_SUCCESS && *user != NULL)
RETURNC(PAM_SUCCESS);

12
lib/libpam/pam_getenv.c
View file

@ -61,18 +61,12 @@ pam_getenv(pam_handle_t *pamh,
int i;
ENTERS(name);
if (pamh == NULL)
RETURNS(NULL);
if (name == NULL || strchr(name, '=') != NULL)
if (strchr(name, '=') != NULL)
RETURNS(NULL);
if ((i = openpam_findenv(pamh, name, strlen(name))) < 0)
RETURNS(NULL);
for (str = pamh->env[i]; *str != '\0'; ++str) {
if (*str == '=') {
++str;
break;
}
}
if ((str = strchr(pamh->env[i], '=')) == NULL)
RETURNS("");
RETURNS(str);
}

2
lib/libpam/pam_getenvlist.c
View file

@ -60,8 +60,6 @@ pam_getenvlist(pam_handle_t *pamh)
int i;
ENTER();
if (pamh == NULL)
RETURNP(NULL);
envlist = malloc(sizeof(char *) * (pamh->env_count + 1));
if (envlist == NULL) {
openpam_log(PAM_LOG_ERROR, "%s",

11
lib/libpam/pam_putenv.c
View file

@ -58,14 +58,13 @@ pam_putenv(pam_handle_t *pamh,
const char *namevalue)
{
char **env, *p;
size_t env_size;
int i;
ENTER();
if (pamh == NULL)
RETURNC(PAM_SYSTEM_ERR);
/* sanity checks */
if (namevalue == NULL || (p = strchr(namevalue, '=')) == NULL)
if ((p = strchr(namevalue, '=')) == NULL)
RETURNC(PAM_SYSTEM_ERR);
/* see if the variable is already in the environment */
@ -79,12 +78,12 @@ pam_putenv(pam_handle_t *pamh,
/* grow the environment list if necessary */
if (pamh->env_count == pamh->env_size) {
env = realloc(pamh->env,
sizeof(char *) * (pamh->env_size * 2 + 1));
env_size = pamh->env_size * 2 + 1;
env = realloc(pamh->env, sizeof(char *) * env_size);
if (env == NULL)
RETURNC(PAM_BUF_ERR);
pamh->env = env;
pamh->env_size = pamh->env_size * 2 + 1;
pamh->env_size = env_size;
}
/* add the variable at the end */

2
lib/libpam/pam_set_data.c
View file

@ -64,8 +64,6 @@ pam_set_data(pam_handle_t *pamh,
pam_data_t *dp;
ENTERS(module_data_name);
if (pamh == NULL)
RETURNC(PAM_SYSTEM_ERR);
for (dp = pamh->module_data; dp != NULL; dp = dp->next) {
if (strcmp(dp->name, module_data_name) == 0) {
if (dp->cleanup)

2
lib/libpam/pam_set_item.c
View file

@ -64,8 +64,6 @@ pam_set_item(pam_handle_t *pamh,
size_t nsize, osize;
ENTERI(item_type);
if (pamh == NULL)
RETURNC(PAM_SYSTEM_ERR);
slot = &pamh->item[item_type];
osize = nsize = 0;
switch (item_type) {

4
lib/libpam/pam_setenv.c
View file

@ -65,11 +65,9 @@ pam_setenv(pam_handle_t *pamh,
int r;
ENTER();
if (pamh == NULL)
RETURNC(PAM_SYSTEM_ERR);
/* sanity checks */
if (name == NULL || value == NULL || strchr(name, '=') != NULL)
if (*name == '\0' || strchr(name, '=') != NULL)
RETURNC(PAM_SYSTEM_ERR);
/* is it already there? */

32
t/t_openpam_dispatch.c
View file

@ -47,26 +47,6 @@
const char *pam_return_so;
T_FUNC(null, "null handle")
{
int pam_err;
#if __clang__
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wnonnull"
#elif __GNUC__
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wnonnull"
#endif
pam_err = pam_authenticate(NULL, 0);
#if __clang__
#pragma clang diagnostic pop
#elif __GNUC__
#pragma GCC diagnostic pop
#endif
return (pam_err == PAM_SYSTEM_ERR);
}
T_FUNC(empty_policy, "empty policy")
{
struct t_pam_conv_script script;
@ -81,7 +61,10 @@ T_FUNC(empty_policy, "empty policy")
tf = t_fopen(NULL);
t_fprintf(tf, "# empty policy\n");
pam_err = pam_start(tf->name, "test", &pamc, &pamh);
t_verbose("pam_start() returned %d\n", pam_err);
if (pam_err != PAM_SUCCESS) {
t_verbose("pam_start() returned %d\n", pam_err);
return (0);
}
/*
* Note: openpam_dispatch() currently returns PAM_SYSTEM_ERR when
* the chain is empty, it should possibly return PAM_SERVICE_ERR
@ -156,9 +139,11 @@ T_FUNC(mod_return, "module return value")
pam_err_name[tc->mod[j].modret]);
}
pam_err = pam_start(tf->name, "test", &pamc, &pamh);
t_verbose("pam_start() returned %d\n", pam_err);
if (pam_err != PAM_SUCCESS)
if (pam_err != PAM_SUCCESS) {
t_verbose("pam_start() returned %d\n", pam_err);
t_fclose(tf);
continue;
}
switch (tc->primitive) {
case PAM_SM_AUTHENTICATE:
pam_err = pam_authenticate(pamh, tc->flags);
@ -192,7 +177,6 @@ T_FUNC(mod_return, "module return value")
*/
static struct t_test *t_plan[] = {
T(null),
T(empty_policy),
T(mod_return),