Thread: Lock conflict behavior?
Hi,
I'm wondering if following behavior of PostgreSQL regarding lock
conflict is an expected one. Here's a scenario:
Session A:BEGIN;SELECT * FROM pg_class limit 1; -- acquires access share lock
Session B:BEGIN;ALTER TABLE pg_class ....; -- waits for acquiring access exclusive lock(wil fail
anywaythough)
Session C:SELECT * FROM pg_class...; -- whatever query which needs to acces pg_class will be
blocked, too bad...
I understand that B should wait for aquiring lock, but Should C wait
for?
Also, it seems that an attacker could do a denial service attack if he
could open session A and B, since other users on session C or
following sessions will be blocked.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
Tatsuo Ishii <ishii@postgresql.org> writes:
> I'm wondering if following behavior of PostgreSQL regarding lock
> conflict is an expected one. Here's a scenario:
> Session A:
> BEGIN;
> SELECT * FROM pg_class limit 1; -- acquires access share lock
> Session B:
> BEGIN;
> ALTER TABLE pg_class ....; -- waits for acquiring access
> exclusive lock(wil fail anyway though)
> Session C:
> SELECT * FROM pg_class...; -- whatever query which needs
> to acces pg_class will be
> blocked, too bad...
> I understand that B should wait for aquiring lock, but Should C wait
> for?
If we didn't do this, then a would-be acquirer of exclusive lock would
have a very serious problem with lock starvation: it might wait forever
in the face of a continuous stream of access-share lock requests.
regards, tom lane
Tom Lane wrote: > Tatsuo Ishii <ishii@postgresql.org> writes: >> I'm wondering if following behavior of PostgreSQL regarding lock >> conflict is an expected one. Here's a scenario: > >> Session A: >> BEGIN; >> SELECT * FROM pg_class limit 1; -- acquires access share lock > >> Session B: >> BEGIN; >> ALTER TABLE pg_class ....; -- waits for acquiring access >> exclusive lock(wil fail anyway though) >> Session C: >> SELECT * FROM pg_class...; -- whatever query which needs >> to acces pg_class will be >> blocked, too bad... > >> I understand that B should wait for aquiring lock, but Should C wait >> for? > > If we didn't do this, then a would-be acquirer of exclusive lock would > have a very serious problem with lock starvation: it might wait forever > in the face of a continuous stream of access-share lock requests. See http://en.wikipedia.org/wiki/Readers-writers_problem Jan -- Jan Urbanski GPG key ID: E583D7D2 ouden estin
On Mon, 2008-12-22 at 17:14 +0900, Tatsuo Ishii wrote: > Also, it seems that an attacker could do a denial service attack if he > could open session A and B, since other users on session C or > following sessions will be blocked. LOCK TABLE checks the permissions before attempting to acquire the lock, is there a reason that ALTER TABLE doesn't? Even if they don't have any rights to the table at all (not even SELECT), there are still other problems. For instance, the user could just wait for a long running query (or VACUUM) and issue the ALTER TABLE at that time. I know we don't make any guarantees about preventing denial-of-service attacks from users that can connect, but if possible we should be consistent about checking the permissions. Regards,Jeff Davis
> > Also, it seems that an attacker could do a denial service attack if he > > could open session A and B, since other users on session C or > > following sessions will be blocked. > > LOCK TABLE checks the permissions before attempting to acquire the lock, > is there a reason that ALTER TABLE doesn't? Right. I think we should check the permissions first too. > Even if they don't have any rights to the table at all (not even > SELECT), there are still other problems. For instance, the user could > just wait for a long running query (or VACUUM) and issue the ALTER TABLE > at that time. In the scenario I mentioned, even a new connection cannot be made to the database since the backend need to initialize relcache by reading system catlogs with access share lock at the very eary stage in strating up. -- Tatsuo Ishii SRA OSS, Inc. Japan
> > > Also, it seems that an attacker could do a denial service attack if he > > > could open session A and B, since other users on session C or > > > following sessions will be blocked. > > > > LOCK TABLE checks the permissions before attempting to acquire the lock, > > is there a reason that ALTER TABLE doesn't? > > Right. I think we should check the permissions first too. > > > Even if they don't have any rights to the table at all (not even > > SELECT), there are still other problems. For instance, the user could > > just wait for a long running query (or VACUUM) and issue the ALTER TABLE > > at that time. > > In the scenario I mentioned, even a new connection cannot be made to > the database since the backend need to initialize relcache by reading > system catlogs with access share lock at the very eary stage in > strating up. Another concern is, two phase commit. If a 2PC transaction includes DDL, access share lock for pg_class is left. Someone comes with alter table pg_class and tries to hold an exclusive lock. After this all SELECT and autovacuum will stop because access share lock for pg_class cannot be aquired. -- Tatsuo Ishii SRA OSS, Inc. Japan
Tatsuo Ishii <ishii@postgresql.org> writes:
>> LOCK TABLE checks the permissions before attempting to acquire the lock,
>> is there a reason that ALTER TABLE doesn't?
> Right. I think we should check the permissions first too.
I've always thought that it was extremely shaky for LOCK to try to work
that way. With no lock, you have no confidence that the table isn't
changing or disappearing under you. In the worst case, the permissions
check might fail outright (likely with a "cache lookup failed" message
about a catalog row that disappeared as we attempted to fetch it); or it
might give an answer that's obsolete by the time we do acquire the lock.
The present coding of LOCK dates from before we had fixed things to
guarantee that relation_open() acquires table lock before attempting to
load the relcache entry. Back then, there were enough race conditions
involved in first access to a relation that one more didn't seem to
matter. But now, I'd sure insist on someone finding a more bulletproof
answer before I'd hold still for extending that bogus coding pattern
all over the system.
regards, tom lane
On Tue, 2008-12-23 at 08:48 -0500, Tom Lane wrote: > I've always thought that it was extremely shaky for LOCK to try to work > that way. With no lock, you have no confidence that the table isn't > changing or disappearing under you. In the worst case, the permissions > check might fail outright (likely with a "cache lookup failed" message > about a catalog row that disappeared as we attempted to fetch it); or it > might give an answer that's obsolete by the time we do acquire the lock. It looks like it would be easy enough to throw a better error message than that, e.g. with a try/catch. The information could be obsolete, but if it succeeds, it would at least mean they had permissions at some time in the past. Or, we could just remove the ACL checks from LOCK TABLE, so that it's at least consistent. Mostly it's the inconsistency that bothers me. Regards,Jeff Davis
Jeff Davis wrote: > On Tue, 2008-12-23 at 08:48 -0500, Tom Lane wrote: > > I've always thought that it was extremely shaky for LOCK to try to work > > that way. With no lock, you have no confidence that the table isn't > > changing or disappearing under you. In the worst case, the permissions > > check might fail outright (likely with a "cache lookup failed" message > > about a catalog row that disappeared as we attempted to fetch it); or it > > might give an answer that's obsolete by the time we do acquire the lock. > > It looks like it would be easy enough to throw a better error message > than that, e.g. with a try/catch. The information could be obsolete, but > if it succeeds, it would at least mean they had permissions at some time > in the past. > > Or, we could just remove the ACL checks from LOCK TABLE, so that it's at > least consistent. Mostly it's the inconsistency that bothers me. Is this a TODO? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On Wed, 2009-01-21 at 17:39 -0500, Bruce Momjian wrote: > > It looks like it would be easy enough to throw a better error message > > than that, e.g. with a try/catch. The information could be obsolete, but > > if it succeeds, it would at least mean they had permissions at some time > > in the past. > > > > Or, we could just remove the ACL checks from LOCK TABLE, so that it's at > > least consistent. Mostly it's the inconsistency that bothers me. > > Is this a TODO? I don't feel too strongly about it. I would feel better if we were consistent about the permissions checks, because there's less of a chance for confusion or a false sense of security. If we keep the permission check in LockTableCommand(), I can make a patch that produces a more useful error message when the table is removed right before the pg_class_aclcheck(). Right now it does: ERROR: relation with OID 16542 does not exist which is undesirable. Regards,Jeff Davis
On Wed, 2009-01-21 at 15:08 -0800, Jeff Davis wrote:
> If we keep the permission check in LockTableCommand(), I can make a
> patch that produces a more useful error message when the table is
> removed right before the pg_class_aclcheck().
Attached.
Regards,
Jeff Davis
Attachment
Jeff Davis <pgsql@j-davis.com> writes:
> On Wed, 2009-01-21 at 15:08 -0800, Jeff Davis wrote:
>> If we keep the permission check in LockTableCommand(), I can make a
>> patch that produces a more useful error message when the table is
>> removed right before the pg_class_aclcheck().
> Attached.
This is pretty horrid, because it converts any error whatsoever into
"relation does not exist". For counterexamples consider "statement
timeout reached", "query cancelled by user", "pg_class is corrupted",
etc etc.
regards, tom lane
On Thu, 2009-01-22 at 18:20 -0500, Tom Lane wrote: > Jeff Davis <pgsql@j-davis.com> writes: > > On Wed, 2009-01-21 at 15:08 -0800, Jeff Davis wrote: > >> If we keep the permission check in LockTableCommand(), I can make a > >> patch that produces a more useful error message when the table is > >> removed right before the pg_class_aclcheck(). > > > Attached. > > This is pretty horrid, because it converts any error whatsoever into > "relation does not exist". For counterexamples consider "statement > timeout reached", "query cancelled by user", "pg_class is corrupted", > etc etc. Ah, I see. Well, I guess there's not a better way to handle that error after all. There's no way to tell what exception you're catching specifically, right? Regards,Jeff Davis