Opinion on checking return value of setuid(getuid())?

[Xin Li]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 10/1/12 3:31 AM, Erik Cederstrand wrote:
> I'm looking through the clang analyzer reports and found this one: 
> http://scan.freebsd.your.org/freebsd-head/sbin.ping/2012-09-30-amd64/report-R9ZgC6.html#EndPath
>
>
> 
It's complaining that, if setuid() fails for some reason, the
> process will continue with root privileges because the process is 
> suid root.
> 
> At first glance, it seems unnecessary to check the return value of 
> "setuid(getuid())" since the user should always be able to drop 
> privileges to itself. So I filed this bug with LLVM: 
> http://llvm.org/bugs/show_bug.cgi?id=13979
> 
> It turns out that setuid() *may* fail if the user hits its process 
> limit. Apparently FreeBSD doesn't check the limit in the specific 
> setuid(getuid()) case (I can't find the code anywhere right now)
> so this is not an issue, but Linux does. However, if FreeBSD
> decides to change the setuid() implementation at some point, the
> issue may surface again.

I didn't follow the idea -- In Linux's kernel/sys.c:

SYSCALL_DEFINE1(setuid, uid_t, uid)
{
(...)
        kuid = make_kuid(ns, uid);
(...)
        if (nsown_capable(CAP_SETUID)) {
                new->suid = new->uid = kuid;
                if (!uid_eq(kuid, old->uid)) { // <-- 1
                        retval = set_user(new); // <-- check done here
                        if (retval < 0)
                                goto error;
                }

How can the check be even reached in setuid(getuid()) case?  It's also
conflict with intuition by the way -- we are not changing ownership of
the process, thus the process number should not change...

Cheers,
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.17 (Darwin)

iQEcBAEBCAAGBQJQaXl0AAoJEG80Jeu8UPuz22AIAIBhAdEscXjcsQR06qzFntn4
lVVLzlPH+KdgUezbE5uMWbtNj0Az7ny66QQ2ocgh5KK8bc5i1486T9+32k6X7Cft
gxE7tpPGkrb6uT62TV4Z5TkJ3NLfqQ6pABiYFONUS72Zy2zPE9stq5X4XrySXlTh
Oft6hpLK5qtxucD7RUKrj8Ofw6kugKm7+KDXqQUU2CuEkCZZUiY1KarJK1fyPHF7
9APaaWyWZt6yMj3qn/2btmR4GZoZMQdfUqe8EIhpxGdKseB81FIdHfDo2bzDGRcx
jUUIbrFxLTypjXws2IPneHYaKpLfs5RWT6yKPRkdKIkfQYTeJMb0MjlD6q7acWo=
=hknO
-----END PGP SIGNATURE-----