Subject: Re: commoning up code that changes uids and gids
To: David Laight <david@l8s.co.uk>
From: Jaromir Dolecek <jdolecek@netbsd.org>
List: tech-kern
Date: 03/04/2003 09:57:36
While commoditation like this is in principle good thing, I don't
think it's wise to do if you are not 200% sure you are right.
E.g. I'm not sure if this in sys_setregid():

> +	if (rgid == -1)
> +		rgid = p->p_cred->p_rgid;
> ...
> +	svgid = rgid == p->p_cred->p_rgid ? -1 : egid;

is bug or uncommented code shortcut or harmless completely.

It is VERY bad to change such sensitive code just for change's
sake, IMHO.

> This also stops the 'compat' functions getting out of step with
> any future changes.
 
Did you confirm the semantics for compat code matches
previous state? E.g. linux_misc.c/linux_misc_notalpha.c has
this comment:

 	/*
 	 * Note: These checks are a little different than the NetBSD
 	 * setreuid(2) call performs.  This precisely follows the
 	 * behavior of the Linux kernel.
 	 */

AFAICS most of other compat code uses some variant of setresuid() playing
with saved IDs; adding compat_setresuid() function to compat/common/
and make compat code use that, should be sufficient WRT code sharing.

Jaromir
-- 
Jaromir Dolecek <jdolecek@NetBSD.org>            http://www.NetBSD.org/
-=- We should be mindful of the potential goal, but as the tantric    -=-
-=- Buddhist masters say, ``You may notice during meditation that you -=-
-=- sometimes levitate or glow.   Do not let this distract you.''     -=-