STABLE14-windows-integrated-logon-hack-fix-for-proper-refcounts-20060119

The Integrated Logon hack of setting a token for a smb name different
than the one associated with the current smb session fails when smb
virtual circuits, sessions and username objects are properly reference
counted.  When refcounts are not leaked the constructed smb_username_t
is destroyed immediately after the token is set since there are not
references to it from a current session.

The fix is to mark the smb_username_t object with a flag indicating that
it was created by the Network Provider.  This flag prevents the destruction
when the refcount is zero so that it will be available at the time the
smb session is created (just a moment or two later.)  During the binding
of the smb_username_t to the smb_vc_t the flag is cleared allowing the
tokens to be destroyed when the smb session is closed.

(cherry picked from commit 70b76b3a1cff1dabe9b10b8222cd84fc207b6704)

diff --git a/src/WINNT/afsd/afslogon.c b/src/WINNT/afsd/afslogon.c
index 8903bfede8f6b9c86bc597af1c8bcefe9f20d08f..9752eb6405c0b5ff57f9ef7bb7c5c028402d16eb 100644 (file)
--- a/src/WINNT/afsd/afslogon.c
+++ b/src/WINNT/afsd/afslogon.c
@@ -801,7 +801,8 @@ DWORD APIENTRY NPLogonNotify(
         {                      
             if ( KFW_is_available() ) {
                 code = KFW_AFS_get_cred(uname, cell, password, 0, opt.smbName, &reason);
-                DebugEvent("KFW_AFS_get_cred  uname=[%s] smbname=[%s] cell=[%s] code=[%d]",uname,opt.smbName,cell,code);
+                DebugEvent("KFW_AFS_get_cred  uname=[%s] smbname=[%s] cell=[%s] code=[%d]",
+                           uname,opt.smbName,cell,code);
                 if (code == 0 && opt.theseCells) { 
                     char * principal, *p;
 
@@ -830,8 +831,8 @@ DWORD APIENTRY NPLogonNotify(
                 code = ka_UserAuthenticateGeneral2(KA_USERAUTH_VERSION+KA_USERAUTH_AUTHENT_LOGON,
                                                     uname, "", cell, password, opt.smbName, 0, &pw_exp, 0,
                                                     &reason);
-                DebugEvent("AFS AfsLogon - (INTEGRATED only)ka_UserAuthenticateGeneral2","Code[%x] uname[%s] Cell[%s]",
-                            code,uname,cell);
+                DebugEvent("AFS AfsLogon - (INTEGRATED only)ka_UserAuthenticateGeneral2 Code[%x] uname[%s] smbname=[%s] Cell[%s] PwExp=[%d] Reason=[%s]",
+                            code,uname,opt.smbName,cell,pw_exp,reason?reason:"");
             }       
             if ( code && code != KTC_NOCM && code != KTC_NOCMRPC && !lowercased_name ) {
                 for ( ctemp = uname; *ctemp ; ctemp++) {

diff --git a/src/WINNT/afsd/cm_ioctl.c b/src/WINNT/afsd/cm_ioctl.c
index a4bfa36a57b1671ce780d1fdab759fae0e2cb2ea..71c463e8281e8020d517adb35ea736411c0fddf7 100644 (file)
--- a/src/WINNT/afsd/cm_ioctl.c
+++ b/src/WINNT/afsd/cm_ioctl.c
@@ -1951,7 +1951,8 @@ long cm_IoctlSetToken(struct smb_ioctl *ioctlp, struct cm_user *userp)
     }
 
     if (flags & PIOCTL_LOGON) {
-        userp = smb_FindCMUserByName(smbname, ioctlp->fidp->vcp->rname);
+        userp = smb_FindCMUserByName(smbname, ioctlp->fidp->vcp->rname,
+                                    SMB_FLAG_CREATE|SMB_FLAG_AFSLOGON);
        release_userp = 1;
     }
 

diff --git a/src/WINNT/afsd/cm_user.c b/src/WINNT/afsd/cm_user.c
index 6d7ba073f51c53579d18e265be66979b55ab9212..afc4c483e7d08eb35f6fe37942ab402816f3e1bb 100644 (file)
--- a/src/WINNT/afsd/cm_user.c
+++ b/src/WINNT/afsd/cm_user.c
@@ -39,14 +39,13 @@ void cm_InitUser(void)
 
 cm_user_t *cm_NewUser(void)
 {
-    cm_user_t *up;
+    cm_user_t *userp;
         
-    up = malloc(sizeof(*up));
-    memset(up, 0, sizeof(*up));
-    up->refCount = 1;
-    up->vcRefs = 1;            /* from caller */
-    lock_InitializeMutex(&up->mx, "cm_user_t");
-    return up;
+    userp = malloc(sizeof(*userp));
+    memset(userp, 0, sizeof(*userp));
+    userp->refCount = 1;
+    lock_InitializeMutex(&userp->mx, "cm_user_t");
+    return userp;
 }
 
 /* must be called with locked userp */
@@ -98,31 +97,39 @@ void cm_HoldUser(cm_user_t *up)
     lock_ReleaseWrite(&cm_userLock);
 }
 
-void cm_ReleaseUser(cm_user_t *up)
+void cm_ReleaseUser(cm_user_t *userp)
 {
     cm_ucell_t *ucp;
     cm_ucell_t *ncp;
 
-    if (up == NULL) 
+    if (userp == NULL) 
         return;
 
     lock_ObtainWrite(&cm_userLock);
-    osi_assert(up->refCount-- > 0);
-    if (up->refCount == 0) {
-        lock_FinalizeMutex(&up->mx);
-        for (ucp = up->cellInfop; ucp; ucp = ncp) {
+    osi_assert(userp->refCount-- > 0);
+    if (userp->refCount == 0) {
+        lock_FinalizeMutex(&userp->mx);
+        for (ucp = userp->cellInfop; ucp; ucp = ncp) {
             ncp = ucp->nextp;
             if (ucp->ticketp) 
                 free(ucp->ticketp);
             free(ucp);
         }
-        free(up);
+        free(userp);
     }
     lock_ReleaseWrite(&cm_userLock);
 }
 
+
+void cm_HoldUserVCRef(cm_user_t *userp)
+{
+    lock_ObtainMutex(&userp->mx);
+    userp->vcRefs++;
+    lock_ReleaseMutex(&userp->mx);
+}       
+
 /* release the count of the # of connections that use this user structure.
- * When this hits zero, we know we won't be getting an new requests from
+ * When this hits zero, we know we won't be getting any new requests from
  * this user, and thus we can start GC'ing connections.  Ref count on user
  * won't hit zero until all cm_conn_t's have been GC'd, since they hold
  * refCount references to userp.

diff --git a/src/WINNT/afsd/cm_user.h b/src/WINNT/afsd/cm_user.h
index 17453e11a9c91d3a4c5fa33a354414f53a6dfb45..f0a2e14697160634bc7a53ee5e3130288a13ef9c 100644 (file)
--- a/src/WINNT/afsd/cm_user.h
+++ b/src/WINNT/afsd/cm_user.h
@@ -48,7 +48,6 @@ typedef struct cm_user {
 } cm_user_t;
 
 #define CM_USERFLAG_DELETE     1       /* delete on last reference */
-#define CM_USERFLAG_WASLOGON   2       /* was logon DLL user */
 
 extern void cm_InitUser(void);
 
@@ -60,6 +59,8 @@ extern cm_ucell_t *cm_FindUCell(cm_user_t *userp, int iterator);
 
 extern void cm_HoldUser(cm_user_t *up);
 
+extern void cm_HoldUserVCRef(cm_user_t *up);
+
 extern void cm_ReleaseUser(cm_user_t *up);
 
 extern void cm_ReleaseUserVCRef(cm_user_t *up);

diff --git a/src/WINNT/afsd/smb.c b/src/WINNT/afsd/smb.c
index 726c18a05f09ee10b149f38e6d167c8b12f1ac40..2139fd8d9bc75b949ee05bd207bb194b43a90ee8 100644 (file)
--- a/src/WINNT/afsd/smb.c
+++ b/src/WINNT/afsd/smb.c
@@ -934,9 +934,9 @@ void smb_CleanupDeadVC(smb_vc_t *vcp)
     smb_tid_t *tidpNext;
     smb_tid_t *tidp;
     unsigned short tid;
-    smb_user_t *userpIter;
-    smb_user_t *userpNext;
-    smb_user_t *userp;
+    smb_user_t *uidpIter;
+    smb_user_t *uidpNext;
+    smb_user_t *uidp;
     unsigned short uid;
     smb_vc_t **vcpp;
 
@@ -982,24 +982,24 @@ void smb_CleanupDeadVC(smb_vc_t *vcp)
        lock_ObtainRead(&smb_rctLock);
     }
 
-    for (userpIter = vcp->usersp; userpIter; userpIter = userpNext) {
-       userpNext = userpIter->nextp;
+    for (uidpIter = vcp->usersp; uidpIter; uidpIter = uidpNext) {
+       uidpNext = uidpIter->nextp;
 
-       if (userpIter->flags & SMB_USERFLAG_DELETE)
+       if (uidpIter->flags & SMB_USERFLAG_DELETE)
            continue;
 
-       uid = userpIter->userID;
-       osi_Log2(smb_logp, "  Cleanup UID %d (userp=0x%x)", uid, userpIter);
+       uid = uidpIter->userID;
+       osi_Log2(smb_logp, "  Cleanup UID %d (uidp=0x%x)", uid, uidpIter);
        lock_ReleaseRead(&smb_rctLock);
 
-       userp = smb_FindUID(vcp, uid, 0);
-       osi_assert(userp);
+       uidp = smb_FindUID(vcp, uid, 0);
+       osi_assert(uidp);
 
-       lock_ObtainMutex(&userp->mx);
-       userp->flags |= SMB_USERFLAG_DELETE;
-       lock_ReleaseMutex(&userp->mx);
+       lock_ObtainMutex(&uidp->mx);
+       uidp->flags |= SMB_USERFLAG_DELETE;
+       lock_ReleaseMutex(&uidp->mx);
 
-       smb_ReleaseUID(userp);
+       smb_ReleaseUID(uidp);
 
        lock_ObtainRead(&smb_rctLock);
     }
@@ -1100,7 +1100,7 @@ smb_user_t *smb_FindUID(smb_vc_t *vcp, unsigned short uid, int flags)
     return uidp;
 }              
 
-smb_username_t *smb_FindUserByName(char *usern, char *machine, int flags)
+smb_username_t *smb_FindUserByName(char *usern, char *machine, afs_uint32 flags)
 {
     smb_username_t *unp= NULL;
 
@@ -1121,7 +1121,10 @@ smb_username_t *smb_FindUserByName(char *usern, char *machine, int flags)
         unp->machine = strdup(machine);
         usernamesp = unp;
         lock_InitializeMutex(&unp->mx, "username_t mutex");
+       if (flags & SMB_FLAG_AFSLOGON)
+           unp->flags = SMB_USERFLAG_AFSLOGON;
     }
+
     lock_ReleaseWrite(&smb_rctLock);
     return unp;
 }      
@@ -1153,7 +1156,7 @@ void smb_ReleaseUsername(smb_username_t *unp)
 
     lock_ObtainWrite(&smb_rctLock);
     osi_assert(unp->refCount-- > 0);
-    if (unp->refCount == 0) {
+    if (unp->refCount == 0 && !(unp->flags & SMB_USERFLAG_AFSLOGON)) {
         lupp = &usernamesp;
         for(up = *lupp; up; lupp = &up->nextp, up = *lupp) {
             if (up == unp) 
@@ -1170,7 +1173,6 @@ void smb_ReleaseUsername(smb_username_t *unp)
     lock_ReleaseWrite(&smb_rctLock);
 
     if (userp) {
-        cm_ReleaseUserVCRef(userp);
         cm_ReleaseUser(userp);
     }  
 }      
@@ -1198,8 +1200,11 @@ void smb_ReleaseUID(smb_user_t *uidp)
     }          
     lock_ReleaseWrite(&smb_rctLock);
 
-    if (unp)
+    if (unp) {
+       if (unp->userp)
+           cm_ReleaseUserVCRef(unp->userp);
        smb_ReleaseUsername(unp);
+    }
 }      
 
 /* retrieve a held reference to a user structure corresponding to an incoming

diff --git a/src/WINNT/afsd/smb.h b/src/WINNT/afsd/smb.h
index 7042dc7db9e0edc310a072466a1ca6ed78371428..4f024521da855e2e8dd0f944bf114289273cb543 100644 (file)
--- a/src/WINNT/afsd/smb.h
+++ b/src/WINNT/afsd/smb.h
@@ -91,6 +91,7 @@ typedef struct smb {
 
 /* flags for functions */
 #define SMB_FLAG_CREATE                1       /* create the structure if necessary */
+#define SMB_FLAG_AFSLOGON       2       /* operating on behalf of afslogon.dll */
 
 /* max # of bytes we'll receive in an incoming SMB message */
 /* the maximum is 2^18-1 for NBT and 2^25-1 for Raw transport messages */
@@ -246,6 +247,7 @@ typedef struct smb_username {
 } smb_username_t;
 
 #define SMB_USERFLAG_DELETE    1       /* delete struct when ref count zero */
+#define SMB_USERFLAG_AFSLOGON   2       /* do not delete when the refCount reaches zero */
 
 #define SMB_MAX_USERNAME_LENGTH 256
 
@@ -499,14 +501,10 @@ extern void smb_ReleaseTID(smb_tid_t *tidp);
 
 extern smb_user_t *smb_FindUID(smb_vc_t *vcp, unsigned short uid, int flags);
 
-extern smb_username_t *smb_FindUserByName(char *usern, char *machine, int flags);
+extern smb_username_t *smb_FindUserByName(char *usern, char *machine, afs_uint32 flags);
 
 extern smb_user_t *smb_FindUserByNameThisSession(smb_vc_t *vcp, char *usern); 
 
-extern smb_username_t *smb_FindUserByName(char *usern, char *machine, int flags);
-
-extern smb_user_t *smb_FindUserByNameThisSession(smb_vc_t *vcp, char *usern);
-
 extern void smb_ReleaseUsername(smb_username_t *unp);
 
 extern void smb_ReleaseUID(smb_user_t *uidp);

diff --git a/src/WINNT/afsd/smb3.c b/src/WINNT/afsd/smb3.c
index 78ab4a077554769baa4a54bbae0d65b38fade740..884014a9c788c00026350a34c4f29705dc873ed6 100644 (file)
--- a/src/WINNT/afsd/smb3.c
+++ b/src/WINNT/afsd/smb3.c
@@ -662,7 +662,6 @@ long smb_ReceiveV3SessionSetupX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *
     smb_user_t *uidp;
     unsigned short newUid;
     unsigned long caps = 0;
-    cm_user_t *userp;
     smb_username_t *unp;
     char *s1 = " ";
     long code = 0; 
@@ -842,20 +841,32 @@ long smb_ReceiveV3SessionSetupX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *
     uidp = smb_FindUserByNameThisSession(vcp, usern);
     if (uidp) {   /* already there, so don't create a new one */
         unp = uidp->unp;
-        userp = unp->userp;
         newUid = uidp->userID;
-        osi_LogEvent("AFS smb_ReceiveV3SessionSetupX",NULL,"FindUserByName:Lana[%d],lsn[%d],userid[%d],name[%s]",vcp->lana,vcp->lsn,newUid,osi_LogSaveString(smb_logp, usern));
-        osi_Log3(smb_logp,"smb_ReceiveV3SessionSetupX FindUserByName:Lana[%d],lsn[%d],userid[%d]",vcp->lana,vcp->lsn,newUid);
+        osi_LogEvent("AFS smb_ReceiveV3SessionSetupX",NULL,"FindUserByName:Lana[%d],lsn[%d],userid[%d],name[%s]",
+                     vcp->lana,vcp->lsn,newUid,osi_LogSaveString(smb_logp, usern));
+        osi_Log3(smb_logp,"smb_ReceiveV3SessionSetupX FindUserByName:Lana[%d],lsn[%d],userid[%d]",
+                 vcp->lana,vcp->lsn,newUid);
         smb_ReleaseUID(uidp);
     }
     else {
-      /* do a global search for the username/machine name pair */
+       cm_user_t *userp;
+
+       /* do a global search for the username/machine name pair */
         unp = smb_FindUserByName(usern, vcp->rname, SMB_FLAG_CREATE);
+       lock_ObtainMutex(&unp->mx);
+       if (unp->flags & SMB_USERFLAG_AFSLOGON) {
+           /* clear the afslogon flag so that the tickets can now 
+            * be freed when the refCount returns to zero.
+            */
+           unp->flags &= ~SMB_USERFLAG_AFSLOGON;
+       }
+       lock_ReleaseMutex(&unp->mx);
 
         /* Create a new UID and cm_user_t structure */
         userp = unp->userp;
         if (!userp)
             userp = cm_NewUser();
+       cm_HoldUserVCRef(userp);
         lock_ObtainMutex(&vcp->mx);
         if (!vcp->uidCounter)
             vcp->uidCounter++; /* handle unlikely wraparounds */
@@ -7176,12 +7187,12 @@ void smb3_Init()
     lock_InitializeMutex(&smb_Dir_Watch_Lock, "Directory Watch List Lock");
 }
 
-cm_user_t *smb_FindCMUserByName(char *usern, char *machine)
+cm_user_t *smb_FindCMUserByName(char *usern, char *machine, afs_uint32 flags)
 {
     smb_username_t *unp;
     cm_user_t *     userp;
 
-    unp = smb_FindUserByName(usern, machine, SMB_FLAG_CREATE);
+    unp = smb_FindUserByName(usern, machine, flags);
     if (!unp->userp) {
         lock_ObtainMutex(&unp->mx);
         unp->userp = cm_NewUser();

diff --git a/src/WINNT/afsd/smb3.h b/src/WINNT/afsd/smb3.h
index c9b5ac443c13e03d36c2c8f1a38dd69d72db4b2d..62a9c92697f17a56b83a4dfd8ea58c0da64aa3f8 100644 (file)
--- a/src/WINNT/afsd/smb3.h
+++ b/src/WINNT/afsd/smb3.h
@@ -192,7 +192,7 @@ extern long smb_ReceiveNTRename(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *
 extern int smb_V3MatchMask(char *namep, char *maskp, int flags);
 
 extern void smb3_Init();
-extern cm_user_t *smb_FindCMUserByName(/*smb_vc_t *vcp,*/ char *usern, char *machine);
+extern cm_user_t *smb_FindCMUserByName(char *usern, char *machine, afs_uint32 flags);
 
 /* SMB auth related functions */
 extern void smb_NegotiateExtendedSecurity(void ** secBlob, int * secBlobLength);