libafs: Avoid using changing unixuser ticket data

PSetTokens was afs_osi_Alloc'ing after afs_osi_Free'ing the previous
token data. This can sleep, causing tu->stp to be pointing to garbage
while we wait to alloc. Additionally, rxkad_NewClientSecurityObject
can sleep while waiting to alloc memory, and so the given tu->stp
pointer given to it by afs_ConnBySA may be invalid by the time it
actually uses the data.

To fix this, we could implement unixuser locking to ensure mutual
exclusion of these events. However, this implements a more
conservative change for the 1.4 and 1.6 branches. In PSetTokens we
alloc the new memory before we change anything, and in afs_ConnBySA we
make copies of the ticket data before giving it to rxkad. With these
changes, the glock gives us enough serialization to avoid issues with
tu->stp changing underneath us.

This change is specific to 1.4 and 1.6. On the master branch, this
issue is fixed by implementing unixuser locks in change
Idd66d72f716b7e7dc08faa31ae43e9a23639bae3.

Reviewed-on: http://gerrit.openafs.org/4649
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementia.org>
(cherry picked from commit 1465946bb6863430bf0efebd024d394549a8775f)

Change-Id: Icab5176bf685c408447f0f32ad65c5b003299d3d
Reviewed-on: http://gerrit.openafs.org/6345
Reviewed-by: Derrick Brashear <shadow@dementix.org>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
This commit is contained in:
Andrew Deason 2011-05-04 12:34:20 -05:00 committed by Derrick Brashear
parent ef65c2e082
commit 8ca56700a8
2 changed files with 35 additions and 14 deletions

View File

@ -72,13 +72,27 @@ afs_pickSecurityObject(struct afs_conn *conn, int *secLevel)
/* Do we have tokens ? */
if (conn->user->vid != UNDEFVID) {
char *ticket;
struct ClearToken ct;
*secLevel = 2;
/* Make a copy of the ticket data to give to rxkad, because the
* the ticket data could change while rxkad is sleeping for memory
* allocation. We should implement locking on unixuser
* structures to fix this properly, but for now, this is easier. */
ticket = afs_osi_Alloc(MAXKTCTICKETLEN);
memcpy(ticket, conn->user->stp, conn->user->stLen);
memcpy(&ct, &conn->user->ct, sizeof(ct));
/* kerberos tickets on channel 2 */
secObj = rxkad_NewClientSecurityObject(
cryptall ? rxkad_crypt : rxkad_clear,
(struct ktc_encryptionKey *)conn->user->ct.HandShakeKey,
conn->user->ct.AuthHandle,
conn->user->stLen, conn->user->stp);
(struct ktc_encryptionKey *)ct.HandShakeKey,
ct.AuthHandle,
conn->user->stLen, ticket);
afs_osi_Free(ticket, MAXKTCTICKETLEN);
}
if (secObj == NULL) {
*secLevel = 0;

View File

@ -1790,9 +1790,9 @@ DECL_PIOCTL(PSetTokens)
struct unixuser *tu;
struct ClearToken clear;
struct cell *tcell;
char *stp;
char *stp, *stpNew;
char *cellName;
int stLen;
int stLen, stLenOld;
struct vrequest treq;
afs_int32 flag, set_parent_pag = 0;
@ -1866,18 +1866,21 @@ DECL_PIOCTL(PSetTokens)
areq = &treq;
}
}
/* now we just set the tokens */
tu = afs_GetUser(areq->uid, i, WRITE_LOCK); /* i has the cell # */
tu->vid = clear.ViceId;
if (tu->stp != NULL) {
afs_osi_Free(tu->stp, tu->stLen);
}
tu->stp = (char *)afs_osi_Alloc(stLen);
if (tu->stp == NULL) {
stpNew = (char *)afs_osi_Alloc(stLen);
if (stpNew == NULL) {
return ENOMEM;
}
memcpy(stpNew, stp, stLen);
/* now we just set the tokens */
tu = afs_GetUser(areq->uid, i, WRITE_LOCK); /* i has the cell # */
stp = tu->stp;
stLenOld = tu->stLen;
tu->vid = clear.ViceId;
tu->stp = stpNew;
tu->stLen = stLen;
memcpy(tu->stp, stp, stLen);
tu->ct = clear;
#ifndef AFS_NOSTATS
afs_stats_cmfullperf.authent.TicketUpdates++;
@ -1891,6 +1894,10 @@ DECL_PIOCTL(PSetTokens)
afs_NotifyUser(tu, UTokensObtained);
afs_PutUser(tu, WRITE_LOCK);
if (stp) {
afs_osi_Free(stp, stLenOld);
}
return 0;
nocell: