[Toybox] [PATCH] Remove unused variable in toys/id.c

[Rob Landley]

On 03/13/2012 02:39 AM, Georgi Chorbadzhiyski wrote:
> On 13.3.2012 г. 03:59, Rob Landley wrote:
>> On 02/27/2012 06:26 PM, Georgi Chorbadzhiyski wrote:
>>> The attached patch fixes compiler warning by removing an unused but set
>>> variable.
>>>
>>>    toys/id.c: In function 'id_main':
>>>    toys/id.c:63:8: warning: variable 'gid' set but not used
>>> [-Wunused-but-set-variable]
>>
>> Ok, this is the oldest pending patch I have from you, and the right fix
>> is to make the code use the actual process gid rather than the
>> /etc/passwd entry.  (The setgid bit or setgid() from root could change
>> that.)
>>
>> Tightened up the code a bit while I was there.  (There's more cleanup to
>> do, but I can get back to that later...)
> 
> I send a patch that I think fixed in properly, here:
> [PATCH] toys/id: Fix output formating and ask for correct group id.
> Message-Id: <1331211194-16335-1-git-send-email-gf at unixsol.org>

Ah, I missed the hunk that updates the help text.  I think I got the
other two issues.

I wound up fairly extensively rewriting the command: it didn't have the
egid= and euid= bits the standard requires, it had a lot of redundancy
in it, pretty_print() was only called from one place so why was it
out-of-line... (according to bloatcheck I shrank the code 81 bytes.
That's not why I did it, but I try not to increase the size when I clean
up code.)

The euid/egid stuff the standard requires is actually fairly hard to
test, because it requires a wrapper binary with the suid bit set.

The way suid works is that it sets the _effective_ user id (and sgid
sets the effective group ID), but not the _real_ uid.  This gives you
the capabilities of root, but lets a command see "ah, I'm acting on
behalf of uid X".

This is how "mount" can check /etc/fstab and see if there are mounts
annotated "user" that normal users can mount/umount. It's how sudo can
prompt you for the right user's password, but then act as root.

The sudo command does the cleanup so the commands it executes are fully
running as root, with both euid and uid both set to 0.  (I.E. it calls
setuid(0);  Or possibly setreuid() or even setresuid();  Don't get me
started about setfsuid(), that's a linux-specific hack for samba.)

Some things cleanup the other way: if you create a setuid shell script
and have the shebang point to #!/bin/bash, by default bash will
setreuid(getuid(), getuid()) and drop permissions.  To prevent it from
doing this, you must supply the -p option on the bash command line,
which is not in the option list in the bash man page...

So yeah, subtle.  I have to tackle the test suite at some point.  Doing
automatic regression testing for mount is also going to require either
qemu, user mode linux, or container support...

Thanks,

Rob

 1331658503.0