From d6c5cde26e35498e7eb7cde02b30ed94d26b37e6 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Fri, 12 Aug 2011 14:50:26 -0500 Subject: [PATCH] LINUX: Revert group changes on keyring failure On Linux kernels that support keyrings, when we setpag we try to add the PAG to the session keyring and to the supplemental group list. Currently, if we fail to add the PAG to the keyring (which may happen due to key quotas, or possibly other reasons), we return failure but the group list is still modified with the new PAG in it. Therefore, if the keyring-based approach fails, the new PAG may still be in use, but there are no keyring keys associated with that PAG, so the PAG may never get destroyed. This can cause a large number of PAGs to accumulate over time, causing performance problems. So, change this so that, in the event that keyring installation fails, we revert the group list back to what it was before we touched it. Also mark all unixusers with the new PAG as expired, in case one got created during processing. Thus, the new PAG never gets used. Reviewed-on: http://gerrit.openafs.org/5238 Tested-by: Derrick Brashear Reviewed-by: Derrick Brashear (cherry picked from commit ee2fbffb04bb8b5098354646e262afa90c1b6f59) Change-Id: Ie954ce2b1bc502cc1abe2fa1eecc18b31d066038 Reviewed-on: http://gerrit.openafs.org/5712 Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear --- src/afs/LINUX/osi_groups.c | 41 ++++++++++++++++++++++++---------- src/afs/LINUX/osi_prototypes.h | 2 +- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/afs/LINUX/osi_groups.c b/src/afs/LINUX/osi_groups.c index 144f0e32c..6c727f302 100644 --- a/src/afs/LINUX/osi_groups.c +++ b/src/afs/LINUX/osi_groups.c @@ -141,7 +141,7 @@ afs_setgroups(cred_t **cr, struct group_info *group_info, int change_parent) int __setpag(cred_t **cr, afs_uint32 pagvalue, afs_uint32 *newpag, - int change_parent) + int change_parent, struct group_info **old_groups) { struct group_info *group_info; struct group_info *tmp; @@ -152,12 +152,16 @@ __setpag(cred_t **cr, afs_uint32 pagvalue, afs_uint32 *newpag, *newpag = (pagvalue == -1 ? genpag() : pagvalue); afs_linux_pag_to_groups(*newpag, group_info, &tmp); - put_group_info(group_info); - group_info = tmp; + if (old_groups) { + *old_groups = group_info; + } else { + put_group_info(group_info); + group_info = NULL; + } - afs_setgroups(cr, group_info, change_parent); + afs_setgroups(cr, tmp, change_parent); - put_group_info(group_info); + put_group_info(tmp); return 0; } @@ -235,10 +239,11 @@ setpag(cred_t **cr, afs_uint32 pagvalue, afs_uint32 *newpag, int change_parent) { int code; + struct group_info *old_groups = NULL; AFS_STATCNT(setpag); - code = __setpag(cr, pagvalue, newpag, change_parent); + code = __setpag(cr, pagvalue, newpag, change_parent, &old_groups); #ifdef LINUX_KEYRING_SUPPORT if (code == 0 && afs_cr_rgid(*cr) != NFSXLATOR_CRED) { @@ -265,6 +270,18 @@ setpag(cred_t **cr, afs_uint32 pagvalue, afs_uint32 *newpag, } #endif /* LINUX_KEYRING_SUPPORT */ + if (code) { + if (old_groups) { + afs_setgroups(cr, old_groups, change_parent); + put_group_info(old_groups); + old_groups = NULL; + } + if (*newpag > -1) { + afs_MarkUserExpired(*newpag); + *newpag = -1; + } + } + return code; } @@ -290,7 +307,7 @@ afs_xsetgroups(int gidsetsize, gid_t * grouplist) cr = crref(); if (old_pag != NOPAG && PagInCred(cr) == NOPAG) { /* re-install old pag if there's room. */ - code = __setpag(&cr, old_pag, &junk, 0); + code = __setpag(&cr, old_pag, &junk, 0, NULL); } crfree(cr); @@ -321,7 +338,7 @@ afs_xsetgroups32(int gidsetsize, gid_t * grouplist) cr = crref(); if (old_pag != NOPAG && PagInCred(cr) == NOPAG) { /* re-install old pag if there's room. */ - code = __setpag(&cr, old_pag, &junk, 0); + code = __setpag(&cr, old_pag, &junk, 0, NULL); } crfree(cr); @@ -350,7 +367,7 @@ asmlinkage long afs32_xsetgroups(int gidsetsize, gid_t *grouplist) cr = crref(); if (old_pag != NOPAG && PagInCred(cr) == NOPAG) { /* re-install old pag if there's room. */ - code = __setpag(&cr, old_pag, &junk, 0); + code = __setpag(&cr, old_pag, &junk, 0, NULL); } crfree(cr); @@ -387,7 +404,7 @@ afs32_xsetgroups(int gidsetsize, u16 * grouplist) cr = crref(); if (old_pag != NOPAG && PagInCred(cr) == NOPAG) { /* re-install old pag if there's room. */ - code = __setpag(&cr, old_pag, &junk, 0); + code = __setpag(&cr, old_pag, &junk, 0, NULL); } crfree(cr); @@ -422,7 +439,7 @@ afs32_xsetgroups32(int gidsetsize, gid_t * grouplist) cr = crref(); if (old_pag != NOPAG && PagInCred(cr) == NOPAG) { /* re-install old pag if there's room. */ - code = __setpag(&cr, old_pag, &junk, 0); + code = __setpag(&cr, old_pag, &junk, 0, NULL); } crfree(cr); @@ -575,7 +592,7 @@ osi_get_keyring_pag(afs_ucred_t *cred) if (afs_linux_cred_is_current(cred) && ((keyring_pag >> 24) & 0xff) == 'A' && keyring_pag != afs_linux_pag_from_groups(current_group_info())) { - __setpag(&cred, keyring_pag, &newpag, 0); + __setpag(&cred, keyring_pag, &newpag, 0, NULL); } } key_put(key); diff --git a/src/afs/LINUX/osi_prototypes.h b/src/afs/LINUX/osi_prototypes.h index c91500c1d..25204ecd2 100644 --- a/src/afs/LINUX/osi_prototypes.h +++ b/src/afs/LINUX/osi_prototypes.h @@ -91,7 +91,7 @@ extern void afs_fill_inode(struct inode *ip, struct vattr *vattr); extern void osi_keyring_init(void); extern void osi_keyring_shutdown(void); extern int __setpag(cred_t **cr, afs_uint32 pagvalue, afs_uint32 *newpag, - int change_parent); + int change_parent, struct group_info **old_groups); #ifdef LINUX_KEYRING_SUPPORT extern afs_int32 osi_get_keyring_pag(afs_ucred_t *); extern struct key_type key_type_afs_pag; -- 2.39.5