Recent change in directory g+s behavior (bug?)

Somewhere between 2.4.5-pre1 and 2.4.6-pre3, the behavior of the setgid bit on directories has changed:

2.4.5-pre1:

# mkdir foo
# chgrp adm foo
# chmod 2775 foo
# cd foo
# ls -las
total 12
   4 drwxrwsr-x    3 root     adm          4096 Jul  3 01:11 .
   4 drwxr-xr-x    6 root     adm          4096 Jul  3 00:23 ..
# mkdir bar
# ls -las
total 12
   4 drwxrwsr-x    3 root     adm          4096 Jul  3 01:11 .
   4 drwxrwxrwt    6 root     root         4096 Jul  3 00:23 ..
   4 drwxr-sr-x    2 root     adm          4096 Jul  3 01:11 bar

2.4.6-pre3 and later, including pre9:

# mkdir foo
# chgrp adm foo 
# chmod 2775 foo
# cd foo
# ls -las 
total 12
   4 drwxrwsr-x    3 root     adm          4096 Jul  3 01:23 .
   4 drwxr-xr-x    6 root     adm          4096 Jul  3 00:38 ..
# mkdir bar
# ls -las
total 12
   4 drwxrwsr-x    3 root     adm          4096 Jul  3 01:23 .
   4 drwxrwxrwt    6 root     root         4096 Jul  3 00:38 ..
   4 drwxr-xr-x    2 root     adm          4096 Jul  3 01:23 bar

The setgid bit on directories created within g+s directories is no longer being set, which is a marked change of behavior that breaks quite a few things here, most notably shared CVS trees. The group ownership does however still seem to be inherited properly for both directories and files.

I'm seeing a lot of VFS and ext2 work in the ChangeLog during this kernel interval, but nothing specific to this problem. Because of that, I'm assuming this isn't expected behavior since it breaks many real-world systems that I'm familiar with and differs from other Unixes (Unices?).

I apologize for not being able to narrow down the specific kernel revision at the moment, but I will be able to do this soon. Is anyone else seeing this? Also, who would be the best person to talk to about these issues right now (if they're not on lkml)?

Thanks very much,

Ken.
[email protected]

Fsck... Linus, please apply the patch below. That's a bug in
ext2_new_inode() that used to be hidden by redundant code in ext2_mkdir().

Notice that current code in ext2_new_inode() makes no sense at all -
the only reason why gcc doesn't scream bloody murder is that we have (unrelated) S_ISLNK(mode) several lines below.

@@ -417,7 +417,6 @@
 		cpu_to_le32(le32_to_cpu(es->s_free_inodes_count) - 1);
 	mark_buffer_dirty(sb->u.ext2_sb.s_sbh);
 	sb->s_dirt = 1;
-	inode->i_mode = mode;
 	inode->i_uid = current->fsuid;
 	if (test_opt (sb, GRPID))
 		inode->i_gid = dir->i_gid;

@@ -427,6 +426,7 @@
 			mode |= S_ISGID;
 	} else
 		inode->i_gid = current->fsgid;
+	inode->i_mode = mode;
 
 	inode->i_ino = j;
 	inode->i_blksize = PAGE_SIZE;	/* This is the optimal IO size (for stat), not the fs block size */