When changing uid/gid taking off the s-bit is definetaly the RIGHT thing
to do. It's much safer this way.
ie. I'm against that second patch
Andi
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
Andi Gutmans - Computer Science, Israel Institute of Technology
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Web: http://andi.il.eu.org PGP: finger andi@vipe.technion.ac.il
KeyID 62F5661D fingerprint = 1A 87 A2 10 2F EF EF AB 47 E0 4D 42 F5 B5 49 AD
On Sun, 1 Jun 1997, Greg Alexander wrote:
> It _always_ kills the sgid/suid bits on a file, despite the fact that the
> comments indicated that it should only kill them when the uid or gid
> changes. I have included a patch to make this work as the comments would
> indicated.
> However, I feel _VERY_ strongly that this is an incorrect behaviour.
> chown is chown, it changes ownership. chmod is chmod, it changes the
> rights. They are seperate system calls because they do seperate jobs. One
> of the great things about Unix is that you can make it do exactly what you
> tell it without ever having to repeat yourself...usually. But if the only
> way to make chown() act _only_ as chown() involves getting the rights,
> calling chown(), then restoring the rights, there is something very clearly
> wrong here.
> So I'm including two patches. The first one makes the code work as
> the comments indicate and I think are suitable for inclusion into a real
> Linux kernel. The second one makes the code work as it should. I don't
> know what POSIX has to say on this and I don't think it matters. Linux
> should not sacrifice sanity merely to be compliant. There are times where
> the standard is weak and we must be better.
> If these were corrupted in the mail, just E-Mail me and I can
> uuencode or MIME them.
>
> --- open.c.orig Sat May 31 22:23:47 1997
> +++ open.c Sun Jun 1 00:36:32 1997
> @@ -402,7 +402,7 @@
> /*
> * If the owner has been changed, remove the setuid bit
> */
> - if (inode->i_mode & S_ISUID) {
> + if ((user != inode->i_uid) && (inode->i_mode & S_ISUID)) {
> newattrs.ia_mode &= ~S_ISUID;
> newattrs.ia_valid |= ATTR_MODE;
> }
> @@ -412,7 +412,7 @@
> * Don't remove the setgid bit if no group execute bit.
> * This is a file marked for mandatory locking.
> */
> - if (((inode->i_mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP))) {
> + if ((group != inode->i_gid) && (((inode->i_mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)))) {
> newattrs.ia_mode &= ~S_ISGID;
> newattrs.ia_valid |= ATTR_MODE;
> }
> -----------------------------------------------------------------------
>
>
> --- open.c.orig Sat May 31 22:23:47 1997
> +++ open.c Sat May 31 22:24:17 1997
> @@ -399,23 +399,6 @@
> newattrs.ia_uid = user;
> newattrs.ia_gid = group;
> newattrs.ia_valid = ATTR_UID | ATTR_GID | ATTR_CTIME;
> - /*
> - * If the owner has been changed, remove the setuid bit
> - */
> - if (inode->i_mode & S_ISUID) {
> - newattrs.ia_mode &= ~S_ISUID;
> - newattrs.ia_valid |= ATTR_MODE;
> - }
> - /*
> - * If the group has been changed, remove the setgid bit
> - *
> - * Don't remove the setgid bit if no group execute bit.
> - * This is a file marked for mandatory locking.
> - */
> - if (((inode->i_mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP))) {
> - newattrs.ia_mode &= ~S_ISGID;
> - newattrs.ia_valid |= ATTR_MODE;
> - }
> inode->i_dirt = 1;
> if (inode->i_sb && inode->i_sb->dq_op) {
> inode->i_sb->dq_op->initialize(inode, -1);
> @@ -454,23 +437,6 @@
> newattrs.ia_uid = user;
> newattrs.ia_gid = group;
> newattrs.ia_valid = ATTR_UID | ATTR_GID | ATTR_CTIME;
> - /*
> - * If the owner has been changed, remove the setuid bit
> - */
> - if (inode->i_mode & S_ISUID) {
> - newattrs.ia_mode &= ~S_ISUID;
> - newattrs.ia_valid |= ATTR_MODE;
> - }
> - /*
> - * If the group has been changed, remove the setgid bit
> - *
> - * Don't remove the setgid bit if no group execute bit.
> - * This is a file marked for mandatory locking.
> - */
> - if (((inode->i_mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP))) {
> - newattrs.ia_mode &= ~S_ISGID;
> - newattrs.ia_valid |= ATTR_MODE;
> - }
> inode->i_dirt = 1;
> if (inode->i_sb->dq_op) {
> inode->i_sb->dq_op->initialize(inode, -1);
> -----------------------------------------------------------------------
>
> Greg Alexander
> http://www.cia-g.com/~sietch/
> ----
> "I read about monkeys in the encyclopedia as soon as I got home from the
> funeral and I wonder if this one throws turds and masturbates all the time
> like those monkeys saw it the zoo in San Francisco or if witness monkeys are
> more like people."
> -- a character in Orson Scott Card and Kathryn H. Kidd's novel,
>
>