Re: Label switcher function

The attached patch is a revised one.

It provides two hooks; the one informs core PG whether the supplied
function needs to be hooked, or not. the other is an actual hook on
prepare, start, end and abort of function invocations.

typedef bool (*needs_function_call_type)(Oid fn_oid);

typedef void (*function_call_type)(FunctionCallEventType event,
FmgrInfo *flinfo, Datum *private);

The hook prototype was a bit modified since the suggestion from
Robert. Because FmgrInfo structure contain OID of the function,
it might be redundant to deliver OID of the function individually.

Rest of parts are revised according to the comment.

I also fixed up source code comments which might become incorrect.

Thanks,

(2010/11/18 11:30), Robert Haas wrote:
> 2010/11/17 KaiGai Kohei<kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>> I revised my patch as I attached.
>>
>> The hook function is modified and consolidated as follows:
>>
>> typedef enum FunctionCallEventType
>> {
>> FCET_BE_HOOKED,
>> FCET_PREPARE,
>> FCET_START,
>> FCET_END,
>> FCET_ABORT,
>> } FunctionCallEventType;
>>
>> typedef Datum (*function_call_event_type)(Oid functionId,
>> FunctionCallEventType event,
>> Datum event_arg);
>> extern PGDLLIMPORT function_call_event_type function_call_event_hook;
>>
>> Unlike the subject of this e-mail, now it does not focus on only switching
>> security labels during execution of a certain functions.
>> For example, we may use this hook to track certain functions for security
>> auditing, performance tuning, and others.
>>
>> In the case of SE-PgSQL, it shall return BoolGetDatum(true), if the target
>> function is configured as a trusted procedure, then, this invocation will
>> be hooked by fmgr_security_definer. In the first call, it shall compute
>> the security context to be assigned during execution on FCET_PREPARE event.
>> Then, it switches to the computed label on the FCET_START event, and
>> restore it on the FCET_END or ECET_ABORT event.
>
> This seems like it's a lot simpler than before, which is good. It
> looks to me as though there should really be two separate hooks,
> though, one for what is now FCET_BE_HOOKED and one for everything
> else. For FCET_BE_HOOKED, you want a function that takes an Oid and
> returns a bool. For the other event types, the functionId and event
> arguments are OK, but I think you should forget about the save_datum
> stuff and just always pass fcache->flinfo and&fcache->private. The
> plugin can get the effect of save_datum by passing around whatever
> state it needs to hold on to using fcache->private. So:
>
> bool (*needs_function_call_hook)(Oid fn_oid);
> void (*function_call_hook)(Oid fn_oid, FunctionCallEventType event,
> FmgrInfo flinfo, Datum *private);
>
> Another general comment is that you've not done a very complete job
> updating the comments; there are several of them in fmgr.c that are no
> longer accurate. Also, please zap the unnecessary whitespace changes.
>
> Thanks,
>

--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>