Prevent race condition when last atop stops and new one is started

When the last atop incarnation terminates, the atopacctd issues a
cleanup of all accounting files and writes a new'current' file.
When a new atop is started meanwhile, a race condition might be
introduced, which is prevented by a semaphore.

Author: Atoptool

acctproc.c

@@ -180,10 +180,13 @@ struct pacctadm {
 #define	ATOPACCTKEY	3121959
 #define	ATOPACCTTOT	100
 
-struct sembuf	semclaim = {0, -1, SEM_UNDO},
-		semrelse = {0, +1, SEM_UNDO},
-		semdecre = {1, -1, SEM_UNDO},
-		semincre = {1, +1, SEM_UNDO};
+struct sembuf	semclaim     =  {0, -1, SEM_UNDO},
+		semrelse     =  {0, +1, SEM_UNDO},
+		semdecre     =  {1, -1, SEM_UNDO},
+		semincre     =  {1, +1, SEM_UNDO};
+
+struct sembuf	semreglock[] = {{0, -1, SEM_UNDO}, {1, -1, SEM_UNDO}},
+		semunlock    =  {1, +1, SEM_UNDO};
 
 /*
 ** switch on the process-accounting mechanism
@@ -250,7 +253,7 @@ acctswon(void)
  	** when the atopacctd daemon is active on this system,
 	** it should be the preferred way to read the accounting records
 	*/
-	if ( (sempacctpubid = semget(PACCTPUBKEY, 0, 0)) != -1)
+	if ( (sempacctpubid = semget(PACCTPUBKEY, 2, 0)) != -1)
 	{
 		FILE 			*cfp;
 		char    		shadowpath[128];
@@ -259,14 +262,14 @@ acctswon(void)
 		if (! droprootprivs() )
 			mcleanstop(42, "failed to drop root privs\n");
 
-		(void) semop(sempacctpubid, &semclaim, 1);
+		(void) semop(sempacctpubid, semreglock, 2);
 
 		snprintf(shadowpath, sizeof shadowpath, "%s/%s/%s",
 					pacctdir, PACCTSHADOWD, PACCTSHADOWC);
 
 		if ( (cfp = fopen(shadowpath, "r")) )
 		{
-			if (fscanf(cfp, "%ld/%lu",
+			if (fscanf(cfp, "%ld/%ld",
 					&curshadowseq, &maxshadowrec) == 2)
 			{
 				fclose(cfp);
@@ -297,6 +300,8 @@ acctswon(void)
 
 							semop(sempacctpubid,
 								&semrelse, 1);
+							semop(sempacctpubid,
+								&semunlock, 1);
 
 							regainrootprivs();
 							return 1;
@@ -316,20 +321,30 @@ acctswon(void)
                 			{
 						supportflags |= ACCTACTIVE;
 						regainrootprivs();
+						semop(sempacctpubid,
+								&semunlock, 1);
 						return 0;
                 			}
 
                        			(void) close(acctfd);
 				}
+				else
+				{
+					perror("open shadowpath");
+					abort();
+				}
 			}
 			else
 			{
+				fprintf(stderr,
+					"fscanf failed on shadow currency\n");
 				fclose(cfp);
 				maxshadowrec = 0;
 			}
 		}
 
 		(void) semop(sempacctpubid, &semrelse, 1);
+		(void) semop(sempacctpubid, &semunlock, 1);
 	}
 
 	/*
@@ -868,7 +883,8 @@ acctphotoproc(struct tstat *accproc, int nrprocs)
 			api->mem.majflt = acctexp(acctrec.ac_majflt);
 			api->dsk.rio    = acctexp(acctrec.ac_rw);
 
-			strcpy(api->gen.name, acctrec.ac_comm);
+			strncpy(api->gen.name, acctrec.ac_comm, PNAMLEN);
+			api->gen.name[PNAMLEN] = '\0';
 			break;
 
 		   case 3:
@@ -895,7 +911,8 @@ acctphotoproc(struct tstat *accproc, int nrprocs)
 			api->mem.majflt = acctexp(acctrec_v3.ac_majflt);
 			api->dsk.rio    = acctexp(acctrec_v3.ac_rw);
 
-			strcpy(api->gen.name, acctrec_v3.ac_comm);
+			strncpy(api->gen.name, acctrec_v3.ac_comm, PNAMLEN);
+			api->gen.name[PNAMLEN] = '\0';
 			break;
 		}
 	}

atopacctd.c

@@ -74,22 +74,33 @@
 /*
 ** Semaphore-handling
 **
-** Two semaphore groups are created with one semaphore each.
+** Two semaphore groups are created:
 **
 ** The private semaphore (group) specifies the number of atopacctd processes
 ** running (to be sure that only one daemon is active at the time).
 **
-** The public semaphore (group) reflects the number of processes using
-** the process accounting shadow files, i.e. the number of clients. This
-** semaphore starts at a high value and should be decremented whenever a
-** client starts using the shadow files and incremented again whenever that
-** client terminates.
+** The public semaphore group contains two semaphores:
+**
+**   0: the number of processes using the process accounting shadow files,
+**	i.e. the number of (atop) clients. This semaphore starts at a high
+**	value and should be decremented by every (atop) client that starts
+**	using the shadow files and incremented again whenever that (atop)
+**	client terminates.
+**
+**   1:	binary semphore that has to be claimed before using/modifying
+**	semaphore 0 in this group.
 */
 static int		semprv;
+
+
 static int		sempub;
 #define SEMTOTAL        100
 #define	NUMCLIENTS	(SEMTOTAL - semctl(sempub, 0, GETVAL, 0))
 
+struct sembuf   semlocknowait = {1, -1, SEM_UNDO|IPC_NOWAIT},
+                semunlock     = {1, +1, SEM_UNDO};
+
+
 static char		atopacctdversion[] = ATOPVERS;
 static char		atopacctddate[]    = ATOPDATE;
 
@@ -248,18 +259,20 @@ main(int argc, char *argv[])
 		}
 	}
 
-	if ( (sempub = semget(PACCTPUBKEY, 0, 0)) == -1)	// not existing?
+	// create new semaphore group
+	//
+	if ( (sempub = semget(PACCTPUBKEY, 0, 0)) != -1)	// existing?
+		(void) semctl(sempub, 0, IPC_RMID, 0);
+
+	if ( (sempub = semget(PACCTPUBKEY, 2, 0666|IPC_CREAT|IPC_EXCL)) >= 0)
 	{
-		if ( (sempub = semget(PACCTPUBKEY, 1,
-						0666|IPC_CREAT|IPC_EXCL)) >= 0)
-		{
-			(void) semctl(sempub, 0, SETVAL, SEMTOTAL);
-		}
-		else
-		{
-			perror("cannot create public semaphore");
-			exit(3);
-		}
+		(void) semctl(sempub, 0, SETVAL, SEMTOTAL);
+		(void) semctl(sempub, 1, SETVAL, 1);
+	}
+	else
+	{
+		perror("cannot create public semaphore");
+		exit(3);
 	}
 
 	/*
@@ -676,33 +689,39 @@ awaitprocterm(int nfd, int afd, int sfd, char *accountpath,
 	** have been using the shadow files till now and
 	** cleanup has to be performed
 	*/
-	if (NUMCLIENTS == 0)
+	if (semop(sempub, &semlocknowait, 1) == 0) 	// lock succeeded?
 	{
-		/*
-		** did last client just disappeared?
-		*/
-		if (*shadowbusyp)
+		if (NUMCLIENTS == 0)
 		{
 			/*
-			** remove all shadow files
+			** did last client just disappear?
 			*/
-			gcshadows(oldshadowp, (*curshadowp)+1);
-			*oldshadowp = 0;
-			*curshadowp = 0;
-			stotsize    = 0;
-
-			/*
- 			** create new file with sequence 0
-			*/
-			(void) close(sfd);
-
-			sfd = createshadow(*curshadowp);
-			setcurrent(*curshadowp);
-
-			*shadowbusyp = 0;
+			if (*shadowbusyp)
+			{
+				/*
+				** remove all shadow files
+				*/
+				gcshadows(oldshadowp, (*curshadowp)+1);
+				*oldshadowp = 0;
+				*curshadowp = 0;
+				stotsize    = 0;
+	
+				/*
+ 				** create new file with sequence 0
+				*/
+				(void) close(sfd);
+	
+				sfd = createshadow(*curshadowp);
+				setcurrent(*curshadowp);
+	
+				*shadowbusyp = 0;
+			}
+	
+			(void) semop(sempub, &semunlock, 1);
+			return 1;
 		}
 
-		return 1;
+		(void) semop(sempub, &semunlock, 1);
 	}
 
 	*shadowbusyp = 1;