Обсуждение: [HACKERS] pageinspect option to forgo buffer locking?
Hi, Occasionally, when debugging issues, I find it quite useful to be able to do a heap_page_items() on a page that's actually locked exclusively concurrently. E.g. investigating the recent multixact vacuuming issues, it was very useful to attach a debugger to one backend to step through freezing, and display the page in another session. Currently the locking in get_raw_page_internal() prevents that. If it's an option defaulting to off, I don't see why we couldn't allow that to skip locking the page's contents. Obviously you can get corrupted contents that way, but we already allow to pass arbitrary stuff to heap_page_items(). Since pinning wouldn't be changed, there's no danger of the page being moved out from under us. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Nov 9, 2017 at 9:49 AM, Andres Freund <andres@anarazel.de> wrote: > Currently the locking in get_raw_page_internal() prevents that. If it's > an option defaulting to off, I don't see why we couldn't allow that to > skip locking the page's contents. Obviously you can get corrupted > contents that way, but we already allow to pass arbitrary stuff to > heap_page_items(). Since pinning wouldn't be changed, there's no danger > of the page being moved out from under us. +1. I've done things like this before myself. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Nov 9, 2017 at 12:49 PM, Andres Freund <andres@anarazel.de> wrote: > Occasionally, when debugging issues, I find it quite useful to be able > to do a heap_page_items() on a page that's actually locked exclusively > concurrently. E.g. investigating the recent multixact vacuuming issues, > it was very useful to attach a debugger to one backend to step through > freezing, and display the page in another session. > > Currently the locking in get_raw_page_internal() prevents that. If it's > an option defaulting to off, I don't see why we couldn't allow that to > skip locking the page's contents. Obviously you can get corrupted > contents that way, but we already allow to pass arbitrary stuff to > heap_page_items(). Since pinning wouldn't be changed, there's no danger > of the page being moved out from under us. heap_page_items() is, if I remember correctly, not necessarily going to tolerate malformed input very well - I think that's why we restrict all of these functions to superusers. So using it in this way would seem to increase the risk of a server crash or other horrible misbehavior. Of course if we're just dev-testing that doesn't really matter. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On 2017-11-09 12:55:30 -0500, Robert Haas wrote: > On Thu, Nov 9, 2017 at 12:49 PM, Andres Freund <andres@anarazel.de> wrote: > > Occasionally, when debugging issues, I find it quite useful to be able > > to do a heap_page_items() on a page that's actually locked exclusively > > concurrently. E.g. investigating the recent multixact vacuuming issues, > > it was very useful to attach a debugger to one backend to step through > > freezing, and display the page in another session. > > > > Currently the locking in get_raw_page_internal() prevents that. If it's > > an option defaulting to off, I don't see why we couldn't allow that to > > skip locking the page's contents. Obviously you can get corrupted > > contents that way, but we already allow to pass arbitrary stuff to > > heap_page_items(). Since pinning wouldn't be changed, there's no danger > > of the page being moved out from under us. > > heap_page_items() is, if I remember correctly, not necessarily going > to tolerate malformed input very well - I think that's why we restrict > all of these functions to superusers. So using it in this way would > seem to increase the risk of a server crash or other horrible > misbehavior. Of course if we're just dev-testing that doesn't really > matter. You can already pass arbitrary byteas to heap_page_items(), so I don't see how this'd change anything exposure-wise? Or are you thinking that users would continually do this with actual page contents and would be more likely to hit edge cases than if using pg_read_binary_file() or such (which obviously sees an out of date page)? Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Nov 9, 2017 at 12:58 PM, Andres Freund <andres@anarazel.de> wrote: > You can already pass arbitrary byteas to heap_page_items(), so I don't > see how this'd change anything exposure-wise? Or are you thinking that > users would continually do this with actual page contents and would be > more likely to hit edge cases than if using pg_read_binary_file() or > such (which obviously sees an out of date page)? More the latter. It's not really an increase in terms of security exposure, but it might lead to more trouble in practice. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Nov 9, 2017 at 12:58 PM, Andres Freund <andres@anarazel.de> wrote:
>> You can already pass arbitrary byteas to heap_page_items(), so I don't
>> see how this'd change anything exposure-wise? Or are you thinking that
>> users would continually do this with actual page contents and would be
>> more likely to hit edge cases than if using pg_read_binary_file() or
>> such (which obviously sees an out of date page)?
> More the latter. It's not really an increase in terms of security
> exposure, but it might lead to more trouble in practice.
If we do this, I'd suggest exposing it as a separate SQL function
get_raw_page_unlocked() rather than as an option to get_raw_page().
The reasoning is that if we ever allow these functions to be controlled
via GRANT instead of hardwired superuser checks (cf nearby debate about
lo_import/lo_export), one might reasonably consider the unlocked form as
more risky than the locked form, and hence not wish to have to give out
privileges to both at once.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2017-11-09 17:14:11 -0500, Tom Lane wrote: > If we do this, I'd suggest exposing it as a separate SQL function > get_raw_page_unlocked() rather than as an option to get_raw_page(). > > The reasoning is that if we ever allow these functions to be controlled > via GRANT instead of hardwired superuser checks (cf nearby debate about > lo_import/lo_export), one might reasonably consider the unlocked form as > more risky than the locked form, and hence not wish to have to give out > privileges to both at once. Good idea! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Nov 9, 2017 at 2:21 PM, Andres Freund <andres@anarazel.de> wrote: > On 2017-11-09 17:14:11 -0500, Tom Lane wrote: >> If we do this, I'd suggest exposing it as a separate SQL function >> get_raw_page_unlocked() rather than as an option to get_raw_page(). >> >> The reasoning is that if we ever allow these functions to be controlled >> via GRANT instead of hardwired superuser checks (cf nearby debate about >> lo_import/lo_export), one might reasonably consider the unlocked form as >> more risky than the locked form, and hence not wish to have to give out >> privileges to both at once. > > Good idea! I hope that you follow up on this soon. -- Peter Geoghegan