[PATCH xserver] check for elevated privileges not uid=0 (V2)

Antoine Martin antoine at nagafix.co.uk
Mon Oct 10 12:00:27 PDT 2011


[snip]
>> Subject: [PATCH xserver] check for elevated privileges not uid=0 (V3)
Couldn't find any reference to it, so I just removed it.

> You must have messed up from V2 to V3 because the variables ended
> outside the function again.
Oops, (re)fixed.

> Note that in V2 there was missing a blank
> line after the variable declarations.
all instances fixed

[snip]
> +       * If there are saved ID's the process might still be priviledged
> privileged
thx!

> +       * even though the above test succeeded.  If issetugid() and
> I also consider double space after period a typographical mistake, but
> I am not gonna run a crusade here :)
Indeed it was.

> +        if (seteuid(0) != 0) {
> +          privsElevated = FALSE;
> You use != 0 to check for failure here...
>
> +        } else {
> +          if (seteuid(oldeuid) == -1) {
>
> ... but == -1 here. Is there any reason for not being consistent?
That's from the libX11 code I duplicated...
(fixed)

> +            /* XXX ouch, coudn't get back to original uid
> +               what can we do ??? */
>
> If we get to this point, the code in the patch has failed. It should
> apologize deeply and offer your phone number for technical support :)
> Seriously, it should print an "internal error" message of some kind,
> maybe including __function__. Just quitting silently and leave a
> cryptic 127 that might or not propagate to the user does not cut it.
Added a more helpful message. Notes:
* most platforms I can get hold of now have "getresuid" or "issetugid", 
so this is unlikely to fire in the real world
* when it does fire, the chances that setuid(0) does not fail but 
setuid(!=0) does is slim.

>
> +            _exit(127);
Changed to plain exit(127)

> Why do you use _exit() and not exit() here? This is not e.g. a forked
> child that should escape normal clean-up?
Are there are risks in calling the exit hooks as root? I can't see any.

See new patch in separate email.

Thanks!
Antoine


> Best regards,
> Tormod



More information about the xorg-devel mailing list