profile/profile.c bug under (at least) Solaris - 3.0alpha??

[Pierre Belanger]

Hello Andrew,

I'm sending it "To:" you because I found the following message
on the mailing list:

   http://lists.samba.org/pipermail/samba/2002-October/082150.html

The following is true at least under Solaris (all releases) and
perhaps other OS? Linux is probably not affected???

Here's the "bug":

- smbd reports :

  Jan 14 11:55:56 carnaval smbd[23609]: [ID 702911 daemon.error]
   ERROR:  we did not create the shmem (owned by another user)
  Jan 14 11:55:56 carnaval smbd[23609]: [ID 702911 daemon.error]
   ERROR: failed to setup profiling

Here's the reason why, at least under Solaris:

% grep root /etc/passwd
root:x:0:1:Super-User:/:/sbin/sh

root gid is 1 under Solaris (group named "other).

In smbd/server.c , here's how it runs...

   line # 668 : sec_init(); , which does:

         initial_uid = geteuid();
         initial_gid = getegid();

   line # 693:

	gain_root_privilege();
	gain_root_group_privilege();

	Set uid/gid/egid/euid to 0.

   line # 751:

           if (!profile_setup(False)) {
                 DEBUG(0,("ERROR: failed to setup profiling\n"));
                 return -1;

In profile/profile.c

line # 139

   if (shm_ds.shm_perm.cuid != sec_initial_uid() ||
       shm_ds.shm_perm.cgid != sec_initial_gid()) {
   DEBUG(0,("ERROR: we did not create the shmem (owned by ...


So...

"gain_root_group_privilege" does setegid/setgid to 0.
When profile/profile.c line #139 checks the gid who
created the shared memory, shm_ds.shm_perm.cgid = 0
BUT sec_initial_gid() returns 1. So, smbd complains
that it did not create the shared memory.

I propose the following changes to profile/profile.c, see
attached diff file againts SAMBA_3_0. I added a check for
"EEXIST" after creating the shared memory. This could? fix
the race condition if shmget is "atomic". No notes on this
under Solaris. I also don't know if EEXIST exists on "all"
OS. Up to you to add it in there or not.

Another "tiny" bug line ~ # 130 in profile/profile.c (it's
fixed in my diff). Also needs to be fixed in SAMBA_2_2 and
HEAD:

---	if ((long)profile_p == -1) {
+++	if ((long)profile_h == -1) {


Fell free to make moifications to my patch, I do welcome
negative/positive comments ;-)

Cheers,
Pierre B.
-------------- next part --------------
--- profile/profile.c.orig	Tue Jan 14 13:37:18 2003
+++ profile/profile.c	Tue Jan 14 14:24:01 2003
@@ -106,24 +106,38 @@
 	/* try to use an existing key */
 	shm_id = shmget(PROF_SHMEM_KEY, 0, 0);
 	
-	/* if that failed then create one. There is a race condition here
-	   if we are running from inetd. Bad luck. */
+	/* if that failed then create one. */
 	if (shm_id == -1) {
+
+		static BOOL redo = True;
+
 		if (read_only) return False;
 		shm_id = shmget(PROF_SHMEM_KEY, sizeof(*profile_h), 
 				IPC_CREAT | IPC_EXCL | IPC_PERMS);
-	}
-	
-	if (shm_id == -1) {
-		DEBUG(0,("Can't create or use IPC area. Error was %s\n", 
-			 strerror(errno)));
-		return False;
+
+		if (shm_id == -1) {
+
+			/* Check if we might have run into a race condition when running
+			   from inetd. Bad luck. */
+			if ((errno == EEXIST) && (redo == True)) {
+
+				/* Make sure we don't spin forever - prevent OS bug */
+				redo = False;
+				DEBUG(0,("Can't create or use IPC area. Error was %s\n", 
+				 strerror(errno)));
+				DEBUG(0,("Trying again to use IPC area.\n"));
+		  		goto again;
+			} else {
+				DEBUG(0,("Can't create or use IPC area. Error was %s\n", 
+				 strerror(errno)));
+				return False;
+			}
+		}
 	}   
 	
-	
 	profile_h = (struct profile_header *)shmat(shm_id, 0, 
 						   read_only?SHM_RDONLY:0);
-	if ((long)profile_p == -1) {
+	if ((long)profile_h == -1) {
 		DEBUG(0,("Can't attach to IPC area. Error was %s\n", 
 			 strerror(errno)));
 		return False;
@@ -136,8 +150,13 @@
 		return False;
 	}
 
-	if (shm_ds.shm_perm.cuid != sec_initial_uid() || shm_ds.shm_perm.cgid != sec_initial_gid()) {
+	if (shm_ds.shm_perm.cuid != getuid()) {
 		DEBUG(0,("ERROR: we did not create the shmem (owned by another user)\n"));
+		return False;
+	}
+
+	if (shm_ds.shm_perm.cgid != getgid()) {
+		DEBUG(0,("ERROR: we did not create the shmem (owned by another group)\n"));
 		return False;
 	}