Thread

Commits

  1. Allow GRANT on pg_log_backend_memory_contexts().

  1. Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Jeff Davis <pgsql@j-davis.com> — 2021-10-23T19:57:02Z

    Simple patch to implement $SUBJECT attached.
    
    pg_signal_backend seems like the appropriate predefined role, because
    pg_log_backend_memory_contexts() is implemented by a sending signal.
    
    Regards,
    	Jeff Davis
    
    
  2. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Nathan Bossart <bossartn@amazon.com> — 2021-10-23T20:42:29Z

    On 10/23/21, 12:57 PM, "Jeff Davis" <pgsql@j-davis.com> wrote:
    > pg_signal_backend seems like the appropriate predefined role, because
    > pg_log_backend_memory_contexts() is implemented by a sending signal.
    
    This seems reasonable to me.  The stated reason in the original commit
    message for keeping it restricted to superusers is because of the
    denial-of-service risk, but if you've got pg_signal_backend, you can
    already terminate sessions.  The predefined roles documentation notes
    that members of pg_signal_backend cannot signal superuser-owned
    backends, but AFAICT pg_log_backend_memory_contexts() has no such
    restriction at the moment.  Should we add this?
    
    Otherwise, presumably we will need to update func.sgml and the comment
    above pg_log_backend_memory_contexts() in mcxtfuncs.c.
    
    This is unrelated to this patch, but should we also consider opening
    up pg_reload_conf() and pg_rotate_logfile() to members of
    pg_signal_backend?  Those are the other "server signaling functions" I
    see in the docs.
    
    Nathan
    
    
  3. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Michael Paquier <michael@paquier.xyz> — 2021-10-23T23:41:52Z

    On Sat, Oct 23, 2021 at 08:42:29PM +0000, Bossart, Nathan wrote:
    > Otherwise, presumably we will need to update func.sgml and the comment
    > above pg_log_backend_memory_contexts() in mcxtfuncs.c.
    
    Yes, the documentation of any SQL function whose hardcoded superuser()
    check is removed needs a refresh to outline that its execution can be
    GRANT-ed post-initialization, and it should also document which system
    roles are able to use it.  See for instance pg_database_size(), that
    mentions roles need to be a member of pg_read_all_stats.
    
    > This is unrelated to this patch, but should we also consider opening
    > up pg_reload_conf() and pg_rotate_logfile() to members of
    > pg_signal_backend?  Those are the other "server signaling functions" I
    > see in the docs.
    
    Yes, there is that as well.
    
    +CREATE ROLE testrole1 IN ROLE pg_signal_backend;
    +CREATE ROLE testrole2;
    Any role created in the regression test needs to be prefixed with
    "regress_", or builds with -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
    will complain (I just add that by default to not fall into this trap
    again).
    --
    Michael
    
  4. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2021-10-24T14:28:16Z

    On Sun, Oct 24, 2021 at 1:27 AM Jeff Davis <pgsql@j-davis.com> wrote:
    >
    >
    > Simple patch to implement $SUBJECT attached.
    >
    > pg_signal_backend seems like the appropriate predefined role, because
    > pg_log_backend_memory_contexts() is implemented by a sending signal.
    
    +1.
    
    It looks like we are better off with removing explicit superuser()
    checks from the functions and using normal GRANT based system, see
    others agreeing on this at [1]. As we have lots of functions that are
    doing explicit superuser() checks, I'm sure someday they all will have
    to be moved to GRANT system. The current code is a mix - some
    functions do explicit checks (I've seen many of them with the comment
    at [2]) and others do it via GRANT system. I'm not saying that we
    should be dealing with those here in this thread, all I'm looking for
    is that we have a note of it in the postgres todo list in the wiki so
    that someone interested can pick that work up. Thoughts?
    
    Comments on the patch:
    1) testrole1 and testrole2 look generic, how about
    regress_mcxt_role1/2? There's no problem as they are
    misc_functions.sql local, but still role names can be more readable.
    +CREATE ROLE testrole1 IN ROLE pg_signal_backend;
    +CREATE ROLE testrole2;
    2) It seems like the privileges.sql is the right place to place the
    test cases, but I'm fine with keeping all the test cases of the
    function together.
    3) It might be enough to do has_function_privilege, just a thought -
    isn't it better if we execute the function with the test roles set in.
    This will help us catch the permission denied error message in the
    test output files.
    4) Isn't the +#define CATALOG_VERSION_NO 202110230 going to be set to
    the date on which the patch gets committed?
    5) The following change is being handled in the patch at [3], I know
    it is appropriate to have it in this patch, but please mention it in
    the commit message on why we do this change. I will remove this change
    from my patch at [3].
    -SELECT * FROM pg_log_backend_memory_contexts(pg_backend_pid());
    +SELECT pg_log_backend_memory_contexts(pg_backend_pid());
    
    [1] - https://www.postgresql.org/message-id/CAOuzzgpp0dmOFjWC4JDvk57ZQGm8umCrFdR1at4b80xuF0XChw%40mail.gmail.com
    [2] -
     * Permission checking for this function is managed through the normal
     * GRANT system.
     */
    [3] - https://www.postgresql.org/message-id/CALj2ACVXk1roswqFpiCOMHrsB%2BxxW7HG536krGAzF%3DmWXh3eWQ%40mail.gmail.com
    
    Regards,
    Bharath Rupireddy.
    
    
    
    
  5. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Jeff Davis <pgsql@j-davis.com> — 2021-10-24T16:50:58Z

    On Sat, 2021-10-23 at 20:42 +0000, Bossart, Nathan wrote:
    > The predefined roles documentation notes
    > that members of pg_signal_backend cannot signal superuser-owned
    > backends, but AFAICT pg_log_backend_memory_contexts() has no such
    > restriction at the moment.  Should we add this?
    
    Added, good catch.
    
    > This is unrelated to this patch, but should we also consider opening
    > up pg_reload_conf() and pg_rotate_logfile() to members of
    > pg_signal_backend?  Those are the other "server signaling functions"
    > I
    > see in the docs.
    
    Those are actually signalling the postmaster, not an ordinary backend.
    Also, those functions are already GRANTable, so I think we should leave
    them as-is.
    
    Regards,
    	Jeff Davis
    
    
  6. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Jeff Davis <pgsql@j-davis.com> — 2021-10-24T17:04:48Z

    On Sun, 2021-10-24 at 19:58 +0530, Bharath Rupireddy wrote:
    > It looks like we are better off with removing explicit superuser()
    > checks from the functions and using normal GRANT based system, see
    > others agreeing on this at [1]. As we have lots of functions that are
    > doing explicit superuser() checks, I'm sure someday they all will
    > have
    > to be moved to GRANT system.
    
    Note that some functions have additional checks that can't be expressed
    with GRANT -- see pg_cancel_backend(), for example. But I agree in
    general that GRANT is the way to go most of the time.
    
    >  The current code is a mix - some
    > functions do explicit checks (I've seen many of them with the comment
    > at [2]) and others do it via GRANT system. I'm not saying that we
    > should be dealing with those here in this thread, all I'm looking for
    > is that we have a note of it in the postgres todo list in the wiki so
    > that someone interested can pick that work up. Thoughts?
    
    It seems like there's agreement on the direction, but I don't know that
    there's a good place to write it down. Probably better to just fix as
    many of the functions as we can, and then when people add new ones,
    they'll copy the GRANT pattern rather than the explicit superuser
    check.
    
    > Comments on the patch:
    > 1) testrole1 and testrole2 look generic, how about
    
    Michael had a similar comment. Renamed, thank you.
    
    > 2) It seems like the privileges.sql is the right place to place the
    > test cases, but I'm fine with keeping all the test cases of the
    > function together.
    
    If we add all the function privilege checks there, I think it will
    overwhelm the other interesting tests happening in that file.
    
    > 3) It might be enough to do has_function_privilege, just a thought -
    > isn't it better if we execute the function with the test roles set
    > in.
    > This will help us catch the permission denied error message in the
    > test output files.
    
    Missed this comment. I'll tweak this before commit.
    
    > 4) Isn't the +#define CATALOG_VERSION_NO 202110230 going to be set to
    > the date on which the patch gets committed?
    
    I just put it in there so that I wouldn't forget, but I'll update it at
    commit time.
    
    > 5) The following change is being handled in the patch at [3], I know
    > it is appropriate to have it in this patch, but please mention it in
    > the commit message on why we do this change. I will remove this
    > change
    > from my patch at [3].
    > -SELECT * FROM pg_log_backend_memory_contexts(pg_backend_pid());
    > +SELECT pg_log_backend_memory_contexts(pg_backend_pid());
    
    What would you like me to mention?
    
    Regards,
    	Jeff Davis
    
    
    
    
    
    
  7. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Nathan Bossart <bossartn@amazon.com> — 2021-10-24T20:59:48Z

    On 10/24/21, 9:51 AM, "Jeff Davis" <pgsql@j-davis.com> wrote:
    > On Sat, 2021-10-23 at 20:42 +0000, Bossart, Nathan wrote:
    >> The predefined roles documentation notes
    >> that members of pg_signal_backend cannot signal superuser-owned
    >> backends, but AFAICT pg_log_backend_memory_contexts() has no such
    >> restriction at the moment.  Should we add this?
    >
    > Added, good catch.
    
    The new patch looks good to me.
    
    Nathan
    
    
  8. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2021-10-25T02:53:33Z

    At Sun, 24 Oct 2021 09:50:58 -0700, Jeff Davis <pgsql@j-davis.com> wrote in 
    > On Sat, 2021-10-23 at 20:42 +0000, Bossart, Nathan wrote:
    > > The predefined roles documentation notes
    > > that members of pg_signal_backend cannot signal superuser-owned
    > > backends, but AFAICT pg_log_backend_memory_contexts() has no such
    > > restriction at the moment.  Should we add this?
    > 
    > Added, good catch.
    > 
    > > This is unrelated to this patch, but should we also consider opening
    > > up pg_reload_conf() and pg_rotate_logfile() to members of
    > > pg_signal_backend?  Those are the other "server signaling functions"
    > > I
    > > see in the docs.
    > 
    > Those are actually signalling the postmaster, not an ordinary backend.
    > Also, those functions are already GRANTable, so I think we should leave
    > them as-is.
    
    I'm afraid that it might be wrong that all backend-signalling features
    are allowed by that priviledge.  pg_signal_backends is described in
    the doc as:
    
    https://www.postgresql.org/docs/devel/predefined-roles.html
    
    > Signal another backend to cancel a query or terminate its session.
    
    Here, the term "signal" there seems to mean interrupting something on
    that session or the session itself.  Addition to that I don't think
    "terminate a session or the query on a session" and "log something on
    another session" and "rotate log file" don't fall into the same
    category in a severity view.
    
    In other words, I don't think pg_signal_backends is not meant to
    control "log something on another session" or "rotate log file".  It's
    danger that if we allow somewone to rotate log files, that means to
    allow same user to terminate another session.
    
    
    regards.
    
    -- 
    Kyotaro Horiguchi
    NTT Open Source Software Center
    
    
    
    
  9. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Jeff Davis <pgsql@j-davis.com> — 2021-10-25T03:31:37Z

    On Mon, 2021-10-25 at 11:53 +0900, Kyotaro Horiguchi wrote:
    > In other words, I don't think pg_signal_backends is not meant to
    > control "log something on another session" or "rotate log file". 
    > It's
    > danger that if we allow somewone to rotate log files, that means to
    > allow same user to terminate another session.
    
    The current patch doesn't allow members of pg_signal_backend to rotate
    the log file.
    
    Do you think pg_signal_backend is the wrong group to allow usage of
    pg_log_backend_memory_contexts()? Alternatively, it could simply not
    GRANT anything, and leave that up to the administrator to choose who
    can use it.
    
    Regards,
    	Jeff Davis
    
    
    
    
    
    
  10. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2021-10-25T04:13:43Z

    At Sun, 24 Oct 2021 20:31:37 -0700, Jeff Davis <pgsql@j-davis.com> wrote in 
    > On Mon, 2021-10-25 at 11:53 +0900, Kyotaro Horiguchi wrote:
    > > In other words, I don't think pg_signal_backends is not meant to
    > > control "log something on another session" or "rotate log file". 
    > > It's
    > > danger that if we allow somewone to rotate log files, that means to
    > > allow same user to terminate another session.
    > 
    > The current patch doesn't allow members of pg_signal_backend to rotate
    > the log file.
    
    Ah, sorry, I might have confused with some other discussion.
    
    > Do you think pg_signal_backend is the wrong group to allow usage of
    > pg_log_backend_memory_contexts()? Alternatively, it could simply not
    
    Yes. I think it would be danger that who is allowed to dump memory
    context into log files by granting pg_signal_backend also can
    terminate other backends.
    
    > pg_log_backend_memory_contexts()? Alternatively, it could simply not
    > GRANT anything, and leave that up to the administrator to choose who
    > can use it.
    
    *I* prefer that. I'm not sure I'm the only one to think so, though..
    
    regards.
    
    -- 
    Kyotaro Horiguchi
    NTT Open Source Software Center
    
    
    
    
  11. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2021-10-25T04:27:52Z

    On Mon, Oct 25, 2021 at 9:43 AM Kyotaro Horiguchi
    <horikyota.ntt@gmail.com> wrote:
    > > Do you think pg_signal_backend is the wrong group to allow usage of
    > > pg_log_backend_memory_contexts()? Alternatively, it could simply not
    >
    > Yes. I think it would be danger that who is allowed to dump memory
    > context into log files by granting pg_signal_backend also can
    > terminate other backends.
    >
    > > pg_log_backend_memory_contexts()? Alternatively, it could simply not
    > > GRANT anything, and leave that up to the administrator to choose who
    > > can use it.
    >
    > *I* prefer that. I'm not sure I'm the only one to think so, though..
    
    How about we have a separate predefined role for the functions that
    deal with server logs? I'm not sure if Mark Dilger's patch on new
    predefined roles has one, if not, how about something like
    pg_write_server_log/pg_manage_server_log/some other name?
    
    If not with a new predefined role, how about expanding the scope of
    existing pg_write_server_files role?
    
    Regards,
    Bharath Rupireddy.
    
    
    
    
  12. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Michael Paquier <michael@paquier.xyz> — 2021-10-25T07:10:15Z

    On Sun, Oct 24, 2021 at 08:31:37PM -0700, Jeff Davis wrote:
    > The current patch doesn't allow members of pg_signal_backend to rotate
    > the log file.
    > 
    > Do you think pg_signal_backend is the wrong group to allow usage of
    > pg_log_backend_memory_contexts()? Alternatively, it could simply not
    > GRANT anything, and leave that up to the administrator to choose who
    > can use it.
    
    Hmm.  Why don't you split the patch into two parts that can be
    discussed separately then?  There would be one to remove all the
    superuser() checks you can think of, and a potential second to grant 
    those function's execution to some system role.
    
    FWIW, if the barrier between a role and a function is thin, perhaps
    we'd better just limit ourselves to the removal of the superuser()
    checks for now rather than trying to plug more groups into the
    functions.  When I have dealt with such issues in the past, I tend to
    just do the superuser()/REVOKE part without more GRANTs or even more
    system roles, as this is enough to give room to users to do what they
    want with their clusters.  And this is a no-brainer.
    --
    Michael
    
  13. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2021-10-25T09:14:51Z

    On Sun, Oct 24, 2021 at 10:34 PM Jeff Davis <pgsql@j-davis.com> wrote:
    > > 5) The following change is being handled in the patch at [3], I know
    > > it is appropriate to have it in this patch, but please mention it in
    > > the commit message on why we do this change. I will remove this
    > > change
    > > from my patch at [3].
    > > -SELECT * FROM pg_log_backend_memory_contexts(pg_backend_pid());
    > > +SELECT pg_log_backend_memory_contexts(pg_backend_pid());
    >
    > What would you like me to mention?
    
    Something like below in the commit message would be good:
    "While on this, change the way the tests use pg_log_backend_memory_contexts()
    Usually for functions, we don't use "SELECT-FROM-<<function>>",
    we just use "SELECT-<<function>>"."
    
    Regards,
    Bharath Rupireddy.
    
    
    
    
  14. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2021-10-25T09:19:57Z

    On Mon, Oct 25, 2021 at 12:40 PM Michael Paquier <michael@paquier.xyz> wrote:
    >
    > On Sun, Oct 24, 2021 at 08:31:37PM -0700, Jeff Davis wrote:
    > > The current patch doesn't allow members of pg_signal_backend to rotate
    > > the log file.
    > >
    > > Do you think pg_signal_backend is the wrong group to allow usage of
    > > pg_log_backend_memory_contexts()? Alternatively, it could simply not
    > > GRANT anything, and leave that up to the administrator to choose who
    > > can use it.
    >
    > Hmm.  Why don't you split the patch into two parts that can be
    > discussed separately then?  There would be one to remove all the
    > superuser() checks you can think of, and a potential second to grant
    > those function's execution to some system role.
    
    IMO, in this thread we can focus on remvong the
    pg_log_backend_memory_contexts()'s superuser() check and +1 to start a
    separate thread to remove superuser() checks for the other functions
    and REVOKE the permissions in appropriate places, for system functins
    system_functions.sql files, for extension functions, the extension
    installation .sql files. See [1] and [2].
    
    [1] - https://www.postgresql.org/message-id/CALj2ACUhCFSUQmZhiQ%2Bw1kZdJGmhNP2cd1LZS4GVGowyjiqftQ%40mail.gmail.com
    [2] - https://www.postgresql.org/message-id/CAOuzzgpp0dmOFjWC4JDvk57ZQGm8umCrFdR1at4b80xuF0XChw%40mail.gmail.com
    
    Regards,
    Bharath Rupireddy.
    
    
    
    
  15. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Nathan Bossart <bossartn@amazon.com> — 2021-10-25T16:15:20Z

    On 10/25/21, 2:21 AM, "Bharath Rupireddy" <bharath.rupireddyforpostgres@gmail.com> wrote:
    > On Mon, Oct 25, 2021 at 12:40 PM Michael Paquier <michael@paquier.xyz> wrote:
    >> Hmm.  Why don't you split the patch into two parts that can be
    >> discussed separately then?  There would be one to remove all the
    >> superuser() checks you can think of, and a potential second to grant
    >> those function's execution to some system role.
    >
    > IMO, in this thread we can focus on remvong the
    > pg_log_backend_memory_contexts()'s superuser() check and +1 to start a
    > separate thread to remove superuser() checks for the other functions
    > and REVOKE the permissions in appropriate places, for system functins
    > system_functions.sql files, for extension functions, the extension
    > installation .sql files. See [1] and [2].
    
    I like the general idea of removing hard-coded superuser checks first
    and granting execution to predefined roles second.  I don't have any
    strong opinion about what should be done in this thread and what
    should be done elsewhere.
    
    Nathan
    
    
  16. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Andres Freund <andres@anarazel.de> — 2021-10-25T17:29:39Z

    Hi,
    
    On 2021-10-23 12:57:02 -0700, Jeff Davis wrote:
    > Simple patch to implement $SUBJECT attached.
    > 
    > pg_signal_backend seems like the appropriate predefined role, because
    > pg_log_backend_memory_contexts() is implemented by a sending signal.
    
    I like the idea of making pg_log_backend_memory_contexts() more widely
    available. But I think tying it to pg_signal_backend isn't great. It's
    unnecessarily baking in an implementation detail.
    
    Greetings,
    
    Andres Freund
    
    
    
    
  17. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Jeff Davis <pgsql@j-davis.com> — 2021-10-25T20:42:07Z

    On Mon, 2021-10-25 at 16:10 +0900, Michael Paquier wrote:
    > Hmm.  Why don't you split the patch into two parts that can be
    > discussed separately then?  There would be one to remove all the
    > superuser() checks you can think of, and a potential second to grant 
    > those function's execution to some system role.
    
    Good idea. Attached a patch to remove the superuser check on
    pg_log_backend_memory_contexts(), except in the case when trying to log
    memory contexts of a superuser backend.
    
    Using pg_signal_backend does not seem to be universally acceptable, so
    I'll just drop the idea of granting to that predefined role.
    
    Regards,
    	Jeff Davis
    
    
  18. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Nathan Bossart <bossartn@amazon.com> — 2021-10-25T21:26:47Z

    On 10/25/21, 1:43 PM, "Jeff Davis" <pgsql@j-davis.com> wrote:
    > On Mon, 2021-10-25 at 16:10 +0900, Michael Paquier wrote:
    >> Hmm.  Why don't you split the patch into two parts that can be
    >> discussed separately then?  There would be one to remove all the
    >> superuser() checks you can think of, and a potential second to grant
    >> those function's execution to some system role.
    >
    > Good idea. Attached a patch to remove the superuser check on
    > pg_log_backend_memory_contexts(), except in the case when trying to log
    > memory contexts of a superuser backend.
    
    LGTM.
    
    Nathan
    
    
  19. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Andres Freund <andres@anarazel.de> — 2021-10-25T21:30:38Z

    Hi,
    
    On 2021-10-25 13:42:07 -0700, Jeff Davis wrote:
    > Good idea. Attached a patch to remove the superuser check on
    > pg_log_backend_memory_contexts(), except in the case when trying to log
    > memory contexts of a superuser backend.
    
    I don't get the reasoning behind the "except ..." logic. What does this
    actually protect against? A reasonable use case for this feature is is to
    monitor memory usage of all backends, and this restriction practially requires
    to still use a security definer function.
    
    Greetings,
    
    Andres Freund
    
    
    
    
  20. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Jeff Davis <pgsql@j-davis.com> — 2021-10-25T23:28:13Z

    On Mon, 2021-10-25 at 14:30 -0700, Andres Freund wrote:
    > I don't get the reasoning behind the "except ..." logic. What does
    > this
    > actually protect against? A reasonable use case for this feature is
    > is to
    > monitor memory usage of all backends, and this restriction practially
    > requires
    > to still use a security definer function.
    
    Nathan brought it up -- more as a question than a request, so perhaps
    it's not necessary. I don't have a strong opinion about it, but I
    included it to be conservative (easier to relax a privilege than to
    tighten one).
    
    I can cut out the in-function check entirely if there's no objection.
    
    Regards,
    	Jeff Davis
    
    [1] https://postgr.es/m/33F34F0C-BB16-48DE-B125-95D340A54AE8@amazon.com
    
    
    
    
    
  21. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Nathan Bossart <bossartn@amazon.com> — 2021-10-25T23:58:31Z

    On 10/25/21, 4:29 PM, "Jeff Davis" <pgsql@j-davis.com> wrote:
    > On Mon, 2021-10-25 at 14:30 -0700, Andres Freund wrote:
    >> I don't get the reasoning behind the "except ..." logic. What does
    >> this
    >> actually protect against? A reasonable use case for this feature is
    >> is to
    >> monitor memory usage of all backends, and this restriction practially
    >> requires
    >> to still use a security definer function.
    >
    > Nathan brought it up -- more as a question than a request, so perhaps
    > it's not necessary. I don't have a strong opinion about it, but I
    > included it to be conservative (easier to relax a privilege than to
    > tighten one).
    
    I asked about it since we were going to grant execution to
    pg_signal_backend, which (per the docs) shouldn't be able to signal a
    superuser-owned backend.  I don't mind removing this now that the
    pg_signal_backend part is removed.
    
    Nathan
    
    
  22. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2021-10-26T14:32:00Z

    
    On 2021/10/26 5:42, Jeff Davis wrote:
    > On Mon, 2021-10-25 at 16:10 +0900, Michael Paquier wrote:
    >> Hmm.  Why don't you split the patch into two parts that can be
    >> discussed separately then?  There would be one to remove all the
    >> superuser() checks you can think of, and a potential second to grant
    >> those function's execution to some system role.
    > 
    > Good idea. Attached a patch to remove the superuser check on
    > pg_log_backend_memory_contexts(), except in the case when trying to log
    > memory contexts of a superuser backend.
    
    -        Only superusers can request to log the memory contexts.
    +        Only superusers can request to log the memory contexts of superuser
    +        backends.
    
    The description "This function is restricted to superusers by default,
    but other users can be granted EXECUTE to run the function."
    should be added into the docs?
    
    Regards,
    
    -- 
    Fujii Masao
    Advanced Computing Technology Center
    Research and Development Headquarters
    NTT DATA CORPORATION
    
    
    
    
  23. Re: Allow pg_signal_backend members to use pg_log_backend_memory_stats().

    Jeff Davis <pgsql@j-davis.com> — 2021-10-26T21:04:14Z

    On Tue, 2021-10-26 at 23:32 +0900, Fujii Masao wrote:
    > The description "This function is restricted to superusers by
    > default,
    > but other users can be granted EXECUTE to run the function."
    > should be added into the docs?
    
    A similar statement already exists right above the table of functions:
    
    "
    Use of these functions is restricted to superusers by default but
    access may be granted to others using GRANT, with noted exceptions."
    
    
    Committed the version that merely removes the superuser check, and
    revokes from public. Privilege can be granted to non-superusers if
    desired.
    
    Thanks everyone for looking.
    
    Regards,
    	Jeff Davis