Re: [HACKERS] pageinspect option to forgo buffer locking?

2017-11-09 Thread Andres Freund
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


Re: [HACKERS] pageinspect option to forgo buffer locking?

2017-11-09 Thread Tom Lane
Robert Haas  writes:
> On Thu, Nov 9, 2017 at 12:58 PM, Andres Freund  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


Re: [HACKERS] pageinspect option to forgo buffer locking?

2017-11-09 Thread Robert Haas
On Thu, Nov 9, 2017 at 12:58 PM, Andres Freund  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


Re: [HACKERS] pageinspect option to forgo buffer locking?

2017-11-09 Thread Andres Freund
On 2017-11-09 12:55:30 -0500, Robert Haas wrote:
> On Thu, Nov 9, 2017 at 12:49 PM, Andres Freund  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


Re: [HACKERS] pageinspect option to forgo buffer locking?

2017-11-09 Thread Robert Haas
On Thu, Nov 9, 2017 at 12:49 PM, Andres Freund  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


Re: [HACKERS] pageinspect option to forgo buffer locking?

2017-11-09 Thread Peter Geoghegan
On Thu, Nov 9, 2017 at 9:49 AM, Andres Freund  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


[HACKERS] pageinspect option to forgo buffer locking?

2017-11-09 Thread Andres Freund
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