zfs discuss - Dec 2005 - Wierd ZFS interaction with sudo?

I''ve just installed express b28 on my system.  After installing, I
decided to move /opt to a zfs pool home/opt.  After installing
Blastwave''s latest sudo, I did my usual edits of sudoers.
When I attempted to execute sudo ls to test things I got this:

 rtfm:/opt/csw/etc:>sudo ls
sudo: /opt/csw/etc/sudoers is owned by gid 1, should be 0


But /opt/csw/etc/sudoers permissions and ownership are correct:
-r--r-----   1 root     other        641 Dec  9 17:45 sudoers


The only thing I can attribute this to is some odd interaction with zfs
and sudo, as the same version of sudo was installed in a UFS /opt
earlier in the afternoon.

Can anyone shed light on this?

Thanks,

John Weekley
This message posted from opensolaris.org

John Weekley:
>  rtfm:/opt/csw/etc:>sudo ls
> sudo: /opt/csw/etc/sudoers is owned by gid 1, should be 0
> But /opt/csw/etc/sudoers permissions and ownership are correct:
> -r--r-----   1 root     other        641 Dec  9 17:45 sudoers
"other" is gid 1.  sudo seems to be wanting the file to be owned by
gid 0 (which
is "root").  

try:

 # chgrp root /opt/csw/etc/sudoers

?

k.

Thanks
That was the first thing I tried.  The ''other'' group ownership
won''t change.  Solaris isn''t new to me, which is why I was
surprised to see this behavior.

Interestingly enough, if I change the owner AND group things seem to work better
# chown weekleyj:root sudoers
# ls -al sudoers

-rwxr-xr-x   1 weekleyj root         641 Dec  9 17:45 sudoers
# chmod 0440 sudoers
# exit

rtfm:/opt/csw/etc:>sudo ls
pkg-get.conf      pkg-get.conf.csw  sudoers           sudoers.CSW
This message posted from opensolaris.org

John Weekley:
> Thanks
> That was the first thing I tried.  The ''other'' group
ownership won''t change.  Solaris isn''t new to me, which is why
I was surprised to see this behavior.
sorry, i was not trying to patronise you  :)
> Interestingly enough, if I change the owner AND group things seem to work
better
> # chown weekleyj:root sudoers
> # ls -al sudoers
hm, my best guess about this is that chown(..., -1, 0) is broken for ZFS
because "oldva" is not initialised in zfs_vnops.c:2087 (in b27a) when
need_policy is not set.

        if ((mask & AT_GID) && vap->va_gid != oldva.va_gid)
                zp->z_phys->zp_gid = (uint64_t)vap->va_gid;

</random speculation>

k.

On Fri, 2005-12-09 at 19:10, larne wrote:
> John Weekley:
> > Thanks
> > That was the first thing I tried.  The ''other'' group
ownership won''t change.  Solaris isn''t new to me, which is why
I was surprised to see this behavior.
> 
> sorry, i was not trying to patronise you  :)
Not at all.  It''s tough to gauge someone''s skill level via
email :)
> 
> > Interestingly enough, if I change the owner AND group things seem to
work better
> > # chown weekleyj:root sudoers
> > # ls -al sudoers
> 
> hm, my best guess about this is that chown(..., -1, 0) is broken for ZFS
> because "oldva" is not initialised in zfs_vnops.c:2087 (in b27a)
when
> need_policy is not set.
> 
>         if ((mask & AT_GID) && vap->va_gid != oldva.va_gid)
>                 zp->z_phys->zp_gid = (uint64_t)vap->va_gid;
> 
> </random speculation>
Which is better speculation than  mine.  I didn''t get past
"WTF??!!??"

John

>On Fri, 2005-12-09 at 19:10, larne wrote:
>> John Weekley:
>> > Thanks
>> > That was the first thing I tried.  The ''other''
group ownership won''t change.  Solaris isn''t ne
w to me, which is why I was surprised to see this behavior. 
>> 
>> sorry, i was not trying to patronise you  :)
>
>Not at all.  It''s tough to gauge someone''s skill level via
email :)
>
>> 
>> > Interestingly enough, if I change the owner AND group things seem
to work better
>> > # chown weekleyj:root sudoers
>> > # ls -al sudoers
>> 
>> hm, my best guess about this is that chown(..., -1, 0) is broken for
ZFS
>> because "oldva" is not initialised in zfs_vnops.c:2087 (in
b27a) when
>> need_policy is not set.
>> 
>>         if ((mask & AT_GID) && vap->va_gid !=
oldva.va_gid)
>>                 zp->z_phys->zp_gid = (uint64_t)vap->va_gid;
>> 
>> </random speculation>
>
>Which is better speculation than  mine.  I didn''t get past
"WTF??!!??"

When I wrote the privilege implementation I wanted to steer all
filesystems along secpolicy_vnode_setattr *always* and have whatever
but unfortunately they did a copy and paste job on that function and
as we all know code duplication in this manner leads to additional bugs.

The privilege model for ZFS is slightly different because it allows
owner ship to be taken, apparently, but that''s no excuse for
duplicating
and slightly modifying a function.

Casper

> hm, my best guess about this is that chown(..., -1, 0) is broken for ZFS
> because "oldva" is not initialised in zfs_vnops.c:2087 (in b27a)
when
> need_policy is not set.
> 
>         if ((mask & AT_GID) && vap->va_gid != oldva.va_gid)
>                 zp->z_phys->zp_gid = (uint64_t)vap->va_gid;
> 
> </random speculation>
Well done!  You''re exactly right.  Mark Shellenbaum opened a bug report
describing the problem:

6362068 zfs chown/chgrp doesn''t always work

Quoting from the description:

	In zfs_setattr() doesn''t always set the uid/gid due to an
	uninitialized oldva structure.  This will only happen when
	taking owner ship or taking group ownership.
	
Thanks for catching this!

Jeff