From c1223f111908341b8be472fe3416fedea920613a Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Wed, 30 Jul 2014 11:12:39 -0500 Subject: [PATCH] ptserver: Fix RemoveFromSGEntry hentry memcpy In this function, hentry is the "previous" continuation entry that we looked at, and centry is the "current" continuation entry. We keep track of the previous continuation entry in case we need to update its 'next' pointer, which we do if we free one of the continuation entries because it is empty after the removal. So, this memcpy is supposed to copy the current entry to the previous one, but the arguments are flipped, so we just copy zeroes to centry (since hentry is initialized to zeroes early on in the function), and hentry never gets set to anything besides zeroes. The effect of this is that whenever a ptdb entry has more than one continuation entry, and we free up any of them after the first one via RemoveFromSGEntry, the previous continuation entry becomes blanked (though the 'next' pointer should still be correct). This means the membership information for that group is not recorded correctly, as it loses a chunk of the IDs that it is a member of. The reverse mapping should still be intact (the parent groups have a pointer to the sub-group), but the group probably doesn't function correctly. The reason this happened is because of the confusing conversion from bcopy to memcpy. Most of the instances of bcopy/bcmp/bzero/etc were converted (correctly) back in commit c5c521af, but the supergroups implementation was added afterwards, in 8ab7a909, and contained a bcopy reference. This bcopy was converted to memcpy in 58d5f38b, but the argument order was not corrected, causing this bug. To fix this, just flip the first two arguments of the memcpy. Just get rid of the casts here, too, to match the code in the non-supergroups RemoveFromEntry and elsewhere. Reviewed-on: http://gerrit.openafs.org/11340 Tested-by: BuildBot Reviewed-by: D Brashear (cherry picked from commit 2d89d447c8b00a40d3fc559813fe31c177da164b) Change-Id: I78b80cb7b043c9d1562b543906a593a985020b43 Reviewed-on: http://gerrit.openafs.org/11352 Tested-by: BuildBot Reviewed-by: Chas Williams - CONTRACTOR Reviewed-by: Benjamin Kaduk Reviewed-by: Stephan Wiesand --- src/ptserver/ptutils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ptserver/ptutils.c b/src/ptserver/ptutils.c index 9d764fcd5..296971c0c 100644 --- a/src/ptserver/ptutils.c +++ b/src/ptserver/ptutils.c @@ -840,7 +840,7 @@ RemoveFromSGEntry(struct ubik_trans *at, afs_int32 aid, afs_int32 bid) } /* for all coentry slots */ hloc = nptr; nptr = centry.next; - memcpy((char *)¢ry, (char *)&hentry, sizeof(centry)); + memcpy(&hentry, ¢ry, sizeof(centry)); } /* while there are coentries */ return PRNOENT; } -- 2.39.5