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):
>>
>> 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.
> 
> Ok, I thought we'd decided using 0 wasn't right for some reason, but
> I'll do that then.

An objref == 0 may have a special meaning when handled by restore_obj().

But here it's entirely private to security_{checkpoint/restart}_obj().

Oren.