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.
--- 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;
        }
 

Reply via email to