Re: [patch 1/4] Add routine for generating an ID for kernel pointer

[Cyrill Gorcunov <gorcunov@gmail.com>]

On Wed, Dec 28, 2011 at 08:06:55AM -0800, Tejun Heo wrote:
> Hello, Cyrill.
> 
> Just my 2 cents.
> 
> On Fri, Dec 23, 2011 at 04:47:42PM +0400, Cyrill Gorcunov wrote:
> > +unsigned long gen_obj_id(void *ptr, int type)
> > +{
> > +	if (!capable(CAP_SYS_ADMIN) || !ptr)
> > +		return 0;
> > +
> > +	BUG_ON(type >= GEN_OBJ_ID_TYPES);
> > +
> > +	/*
> > +	 * Note the simple XOR is used here not in a sake
> > +	 * of security by any means, but rather to break
> > +	 * an "impression" that such IDs means something
> > +	 * other than a number which can be used for comparison
> > +	 * with another number generated by this helper only.
> > +	 */
> > +	return ((unsigned long)ptr) ^ gen_obj_cookie[type];
> > +}
> 
> To me, XOR & CAP_SYS_ADMIN combination doesn't make much sense.  With
> CAP_SYS_ADMIN, there's no reason for XOR - we can just export
> pointers.  If we plan on removing CAP_SYS_ADMIN restriction down the
> road, XOR doesn't help much.  It's too weak.  The XOR is unnecessary
> with CAP_SYS_ADMIN and useless without it.  It seems pointless to me.
> If we're going down this route, I think doing cryptographically safe
> hash would be much better.
> 

Hi Tejun, thanks for comment! Yes, XOR is useless here in security meaning,
but it simply breaks impression that these generating numbers "mean" somthing
(I remained them as Vasily asked).

I personally fine to simply leave plain pointers here and root-only access
since that is enough for us (and our tool will require root privileges
anyway :)

OTOH, we could add some sha2 here with pointer+cookie as an initial value but I fear
this will bring more code comlexity and computing sha2 hash is not that
fast operation, which should be taken into account (note on x86-32 since
pointers are 32bit values one could compute prehash for all space covered
and if an attacker will know somehow cookie value the hash will be easily
broken, not sure if it's really usefull for someone, since if you have root
access to the machine such IDs will be the last thing attacker should be
interested in :)

And it seems noone except us need this interface yet, so maybe sticking with
"pointer exported under root-only" would be enough?

	Cyrill