Re: [PATCH 2/4] cr: add generic LSM c/r support (v6)

[Oren Laadan <orenl-RdfvBDnrOixBDgjK7y7TUQ@public.gmane.org>]

Serge E. Hallyn wrote:
> Quoting Oren Laadan (orenl-RdfvBDnrOixBDgjK7y7TUQ@public.gmane.org):
>>> 	oct 19: At checkpoint, we insert the void* security into the
>>> 		objhash.  The first time that we do so, we next write out
>>> 		the string representation of the context to the checkpoint
>>> 		image, along with the value of the objref for the void*
>>> 		security, and insert that into the objhash.  Then at
>>> 		restart, when we read a LSM context, we read the objref
>>> 		which the void* security had at checkpoint, and we then
>>> 		insert the string context with that objref as well.
>> I hoped to see similar comment inlined in the code.
> 
> If we're happy with this approach, then I will add good comments above
> security_checkpoint_obj and security_restore_obj, and above the objhash
> entries.

[...]

>>> +/**
>>> + * security_checkpoint_obj - if first checkpoint of this void* security,
>>> + * then 1. ask the LSM for a string representing the context, 2. checkpoint
>>> + * that string
>>> + * @ctx: the checkpoint context
>>> + * @security: the void* security being checkpointed
>>> + * @sectype: indicates the type of object, because LSMs can (and do) store
>>> + * @secref: We return the objref here
>>> + * different types of data for different types of objects.
>>> + *
>>> + * Returns the objref of the checkpointed ckpt_lsm_string representing the
>>> + * context, or -error on error.
>>> + *
>>> + * This is only used at checkpoint of course.
>>> + */
>>> +int security_checkpoint_obj(struct ckpt_ctx *ctx, void *security,
>>> +				int sectype, int *secref)
>> This function returns 0 for success or a negative error. It should
>> return the @secref instead of passing it by reference (see your
>> description of the return value above !)
>>
>> [...]
> 
> Yes the comment is out of date but the API is imo an improvement.
> Note that SECURITY_CTX_NONE, -1, is a meaningful secref, and at
> the sametime -EPERM, -1, is conceivably a valid error code (though
> at the moment no lsm will return it).
> 
> So I think overloading the secref with error codes is wrong here.

How about #define SECURITY_CTX_NONE 0 ?
it isn't a valid objref anyway.

[...]

Oren.