Re: [HACKERS] why local_preload_libraries does require a separate directory ?

2011-12-05 Thread Tomas Vondra
On 5.12.2011 00:06, Tom Lane wrote:
 Tomas Vondra t...@fuzzy.cz writes:
 On 4.12.2011 22:16, Tom Lane wrote:
 Um ... why would you design it like that?
 
 The backends are added to pg_stat_activity after the auth hook finishes,
 which means possible race conditions (backends executed at about the
 same time don't see each other in pg_stat_activity). So I use an
 exclusive lock that's acquired before reading pg_stat_activity and
 released after the pg_stat_activity is updated.
 That's the only thing the library loaded using local_preload_libraries
 does - it releases the lock.
 
 That's an unbelievably ugly, and dangerous, kluge.  All you need is one
 backend not loading the second library (and remember,
 local_preload_libraries is user-settable) and you've just locked up the
 system.
 
 Why are you using pg_stat_activity for this anyway?  Searching the
 ProcArray seems much safer ... see CountDBBackends for an example.

Thanks, reading ProcArray works fine, although the ProcArrayStruct is
private to procarray.c so I had to create a local copy. That sounds a
bit too fragile I guess - whenever it changes, the extension will break
without a warning.

Tomas

-- 
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] why local_preload_libraries does require a separate directory ?

2011-12-04 Thread Tom Lane
Tomas Vondra t...@fuzzy.cz writes:
 On 3.12.2011 18:53, Tom Lane wrote:
 Security: it lets the DBA constrain which libraries are loadable this way.

 But local_preload_libraries can be set only in postgresql.conf, right?

No.  It's PGC_BACKEND, which means it can be set at connection start by
a client (eg, via PGOPTIONS).

 The problem I'm trying to solve right now is that I do have an extension
 that needs to install two .so libraries - one loaded using
 shared_preload_libraries, one loaded using local_preload_libraries.

Um ... why would you design it like that?

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] why local_preload_libraries does require a separate directory ?

2011-12-04 Thread Tomas Vondra
On 4.12.2011 22:16, Tom Lane wrote:
 Tomas Vondra t...@fuzzy.cz writes:
 On 3.12.2011 18:53, Tom Lane wrote:
 Security: it lets the DBA constrain which libraries are loadable this way.
 
 But local_preload_libraries can be set only in postgresql.conf, right?
 
 No.  It's PGC_BACKEND, which means it can be set at connection start by
 a client (eg, via PGOPTIONS).
 
 The problem I'm trying to solve right now is that I do have an extension
 that needs to install two .so libraries - one loaded using
 shared_preload_libraries, one loaded using local_preload_libraries.
 
 Um ... why would you design it like that?

Well, the purpose of the extension is to limit number of connections by
db/user/IP. It's using pg_stat_activity and auth hook, and the rules are
loaded from a file into a shared memory segment.

So I need to:

1) allocate memory to keep the rules (so it has to be loaded using
   shared_preload_libraries)

2) check the rules (this is done in auth hook) using pg_stat_activity

The backends are added to pg_stat_activity after the auth hook finishes,
which means possible race conditions (backends executed at about the
same time don't see each other in pg_stat_activity). So I use an
exclusive lock that's acquired before reading pg_stat_activity and
released after the pg_stat_activity is updated.

That's the only thing the library loaded using local_preload_libraries
does - it releases the lock.

Tomas

-- 
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] why local_preload_libraries does require a separate directory ?

2011-12-04 Thread Tom Lane
Tomas Vondra t...@fuzzy.cz writes:
 On 4.12.2011 22:16, Tom Lane wrote:
 Um ... why would you design it like that?

 The backends are added to pg_stat_activity after the auth hook finishes,
 which means possible race conditions (backends executed at about the
 same time don't see each other in pg_stat_activity). So I use an
 exclusive lock that's acquired before reading pg_stat_activity and
 released after the pg_stat_activity is updated.
 That's the only thing the library loaded using local_preload_libraries
 does - it releases the lock.

That's an unbelievably ugly, and dangerous, kluge.  All you need is one
backend not loading the second library (and remember,
local_preload_libraries is user-settable) and you've just locked up the
system.

Why are you using pg_stat_activity for this anyway?  Searching the
ProcArray seems much safer ... see CountDBBackends for an example.

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] why local_preload_libraries does require a separate directory ?

2011-12-04 Thread Tomas Vondra
On 5.12.2011 00:06, Tom Lane wrote:
 Tomas Vondra t...@fuzzy.cz writes:
 On 4.12.2011 22:16, Tom Lane wrote:
 Um ... why would you design it like that?
 
 The backends are added to pg_stat_activity after the auth hook finishes,
 which means possible race conditions (backends executed at about the
 same time don't see each other in pg_stat_activity). So I use an
 exclusive lock that's acquired before reading pg_stat_activity and
 released after the pg_stat_activity is updated.
 That's the only thing the library loaded using local_preload_libraries
 does - it releases the lock.
 
 That's an unbelievably ugly, and dangerous, kluge.  All you need is one
 backend not loading the second library (and remember,
 local_preload_libraries is user-settable) and you've just locked up the
 system.

Yes, but I wasn't aware of that - I thought local_preload_libraries is
defined in postgresql.conf so it seemed fine (yes, it was ugly but it
did not seem that dangerous).

 Why are you using pg_stat_activity for this anyway?  Searching the
 ProcArray seems much safer ... see CountDBBackends for an example.

Because I've never user ProcArray before, but I use pg_stat_activity
quite frequently, so it seemed very natural. Thanks for pointing to
ProcArray/CountDBBackends, I'll see how to use that.

Tomas

-- 
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] why local_preload_libraries does require a separate directory ?

2011-12-03 Thread Tom Lane
Tomas Vondra t...@fuzzy.cz writes:
 why the libraries loaded using local_preload_libraries need to be placed
 in a different subdirectory than libraries loaded using
 shared_preload_libraries?

Security: it lets the DBA constrain which libraries are loadable this way.

 I do understand that leaving the users to load whatever libraries they
 want is a bad idea, but when the library is loaded from postgresql.conf
 it should be safe.

We don't have a mechanism that would allow different limitations to be
placed on a GUC variable depending on where the value came from.
To do what you're proposing would require restricting
local_preload_libraries to be superuser-only, which would be a net
decrease in functionality.

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] why local_preload_libraries does require a separate directory ?

2011-12-03 Thread Tomas Vondra
On 3.12.2011 18:53, Tom Lane wrote:
 Tomas Vondra t...@fuzzy.cz writes:
 why the libraries loaded using local_preload_libraries need to be placed
 in a different subdirectory than libraries loaded using
 shared_preload_libraries?

 Security: it lets the DBA constrain which libraries are loadable this way.

But local_preload_libraries can be set only in postgresql.conf, right?
As this file is maintainted by the DBA, so how does this improve the
security?

The problem I'm trying to solve right now is that I do have an extension
that needs to install two .so libraries - one loaded using
shared_preload_libraries, one loaded using local_preload_libraries.

The make install puts both files into $libdir, so I have to copy it to
the right directory.

 I do understand that leaving the users to load whatever libraries they
 want is a bad idea, but when the library is loaded from postgresql.conf
 it should be safe.

 We don't have a mechanism that would allow different limitations to be
 placed on a GUC variable depending on where the value came from.
 To do what you're proposing would require restricting
 local_preload_libraries to be superuser-only, which would be a net
 decrease in functionality.

Now I'm really confused. AFAIK local_preload_libraries can be set only
from postgresql.conf, not by a user. So the check could be quite simple
- is the backend already started? No: don't check the path. Yes: check
that the path starts with '$libdir/plugin'.

Tomas

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers