pgcrypto bug

Bug fix. Patch applied. Thanks.

---------------------------------------------------------------------------

When given oversized key, encrypt/decrypt corrupted
memory. This fixes it. Also a free() was missing.

--
marko

Index: contrib/pgcrypto/px.c
===================================================================
RCS file: /opt/cvs/pgsql/pgsql/contrib/pgcrypto/px.c,v
retrieving revision 1.3
diff -u -r1.3 px.c
--- contrib/pgcrypto/px.c	25 Oct 2001 05:49:20 -0000	1.3
+++ contrib/pgcrypto/px.c	7 Nov 2001 22:33:44 -0000
@@ -88,6 +88,8 @@
memcpy(ivbuf, iv, ivlen);
}

+	if (klen > ks)
+		klen = ks;
keybuf = px_alloc(ks);
memset(keybuf, 0, ks);
memcpy(keybuf, key, klen);
@@ -96,6 +98,7 @@

if (ivbuf)
px_free(ivbuf);
+ px_free(keybuf);

return err;
}

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/users-lounge/docs/faq.html

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Hi all,

attached is a patch which kills a race condition in the current Makefile
system. The Problem occurs if you compile with make -j3 (as many ppl
with Dual CPU Systems will do).
Removing the y.tab.X file is not a good idea as this will trigger
a second compile in the make install stage ...

Bye, Klaus

--
Klaus Naumann
Systems Administration
GMX Aktiengesellschaft
Riesstrasse 17, 80992 München
Telefon +49.89.143 39-0
Telefax +49.89.143 39-100
mailto:knaumann@gmx-ag.de
http://www.gmx.net/

Klaus Naumann <knaumann@gmx-ag.de> writes:

attached is a patch which kills a race condition in the current Makefile
system. The Problem occurs if you compile with make -j3 (as many ppl
with Dual CPU Systems will do).
Removing the y.tab.X file is not a good idea as this will trigger
a second compile in the make install stage ...

Why? y.tab.X are not make targets, only intermediate files, so I don't
see why removing them will create any problem. And I don't see why
turning them into make targets as your patch does is a good idea.

regards, tom lane

On Thu, 8 Nov 2001, Tom Lane wrote:

Klaus Naumann <knaumann@gmx-ag.de> writes:

attached is a patch which kills a race condition in the current Makefile
system. The Problem occurs if you compile with make -j3 (as many ppl
with Dual CPU Systems will do).
Removing the y.tab.X file is not a good idea as this will trigger
a second compile in the make install stage ...

Why? y.tab.X are not make targets, only intermediate files, so I don't
see why removing them will create any problem. And I don't see why
turning them into make targets as your patch does is a good idea.

It is ... the problem is that with the current Makefile there
are always two files which need the .y files
(i.e. preproc.c and preproc.h). If you now do a make -j3,
make will start things in parallel. This means that bison is called twice
(it can even be called the same time which corrupts the output
y.tab.* files) and if you mv the files away the second call
failes as the second mv can't find the file anymore.
Even a test -f y.tab.c && mv doesn't help as I have
seen cases when the race was that bad that test found the
file but mv didn't anymore ...

So the solution is: Turn the bison and mv part into a single
target which get's called once.
This involves making y.tab.c a target. And to avoid a second
compilation on make install it's necessary to not remove
those files (turn mv into cp).

Also I don't see why keeping the y.tab.* files is a problem.
Neither are they big, nor are they installed or anything.
They are like a .o file ...

Later, Klaus

--
Klaus Naumann
Systems Administration
GMX Aktiengesellschaft
Riesstrasse 17, 80992 M�nchen
Telefon +49.89.143 39-0
Telefax +49.89.143 39-100
mailto:knaumann@gmx-ag.de
http://www.gmx.net/

Klaus Naumann <knaumann@gmx-ag.de> writes:

It is ... the problem is that with the current Makefile there
are always two files which need the .y files
(i.e. preproc.c and preproc.h). If you now do a make -j3,
make will start things in parallel.

But they're generated by the *same action*. If make runs the same
action twice in parallel, it's make that is broken. I think your
gripe is directed to the wrong place.

regards, tom lane

On Fri, 9 Nov 2001, Tom Lane wrote:

Klaus Naumann <knaumann@gmx-ag.de> writes:

It is ... the problem is that with the current Makefile there
are always two files which need the .y files
(i.e. preproc.c and preproc.h). If you now do a make -j3,
make will start things in parallel.

But they're generated by the *same action*. If make runs the same
action twice in parallel, it's make that is broken. I think your
gripe is directed to the wrong place.

I don't have any gripe ! I just wanted to help a good (at least
as I thought) project ...

About the other thing:
This is GNU Make version 3.79.1, by Richard Stallman and Roland McGrath.
On Debian/Woody Linux. So well, this make runs these things in
parallel.
Also even if it would be make's fault I don't see what my patch makes
worse. But if you don't want to apply it, you don't apply it.

Later, Klaus

--
Klaus Naumann
Systems Administration
GMX Aktiengesellschaft
Riesstrasse 17, 80992 M�nchen
Telefon +49.89.143 39-0
Telefax +49.89.143 39-100
mailto:knaumann@gmx-ag.de
http://www.gmx.net/

Klaus Naumann <knaumann@gmx-ag.de> writes:

Also even if it would be make's fault I don't see what my patch makes
worse. But if you don't want to apply it, you don't apply it.

Well, as to whether it gets applied or not, I'll defer to Peter
Eisentraut who has done most of the work recently on our configure and
make support. The reason I'm asking all these questions is that I
want to understand what the problem really is. It seems to me that if
we have a problem with these bison invocations then we are likely to
have similar problems elsewhere. We need to understand why it's
unsafe and what the general rule is for avoiding such mistakes in
future.

What bothers me is that you seem to be saying that *any* construct
involving multiple outputs from one rule is unsafe in a parallel make.
That strikes me as a huge restriction, and one that would surely be
mentioned prominently in the gmake manual if it were real. But I can't
find anything that says that.

I think what you are looking at here is a gmake bug, and that you should
report it to the gmake people.

regards, tom lane

Klaus Naumann writes:

attached is a patch which kills a race condition in the current Makefile
system. The Problem occurs if you compile with make -j3 (as many ppl
with Dual CPU Systems will do).
Removing the y.tab.X file is not a good idea as this will trigger
a second compile in the make install stage ...

Well, -j does work if you're using a tarball, but not in a bare CVS tree.

There are a couple of details that I don't like about your patch, but I
can see that the issue gets fixed.

--
Peter Eisentraut peter_e@gmx.net

On Fri, 9 Nov 2001, Peter Eisentraut wrote:

Klaus Naumann writes:

attached is a patch which kills a race condition in the current Makefile
system. The Problem occurs if you compile with make -j3 (as many ppl
with Dual CPU Systems will do).
Removing the y.tab.X file is not a good idea as this will trigger
a second compile in the make install stage ...

Well, -j does work if you're using a tarball, but not in a bare CVS tree.

Why is that - or better, what's changed in the tar ball which makes it
work ?

There are a couple of details that I don't like about your patch, but I
can see that the issue gets fixed.

What are the details ?

Thanks, Klaus

--
Klaus Naumann
Systems Administration
GMX Aktiengesellschaft
Riesstrasse 17, 80992 M�nchen
Telefon +49.89.143 39-0
Telefax +49.89.143 39-100
mailto:knaumann@gmx-ag.de
http://www.gmx.net/

On Fri, 9 Nov 2001, Tom Lane wrote:

Klaus Naumann <knaumann@gmx-ag.de> writes:

Also even if it would be make's fault I don't see what my patch makes
worse. But if you don't want to apply it, you don't apply it.

Well, as to whether it gets applied or not, I'll defer to Peter
Eisentraut who has done most of the work recently on our configure and
make support. The reason I'm asking all these questions is that I
want to understand what the problem really is. It seems to me that if
we have a problem with these bison invocations then we are likely to
have similar problems elsewhere. We need to understand why it's
unsafe and what the general rule is for avoiding such mistakes in
future.

To me it looks like make is calling the target twice as there are
two dependent files.
I think this is due to a dependency of a dependency.
Lemme explain: What I think is, that we enter a directory and make
a parallel build. We want to compile prepare.c , so make calls the
rule to build prepare.c . A second make (because it's parallel)
sees a file which needs prepare.h, so it calls the rule.
At that point the rule is called twice. If it's now doing things
which are not supposed to work in parallel (like moving away
a file) the build fails.
Maybe this is completely wrong - but this is what it looks
like to me ...

What bothers me is that you seem to be saying that *any* construct
involving multiple outputs from one rule is unsafe in a parallel make.

I didn't say that - I just was refering to this special case.
It happened to me - more then one time. Even about 20 times
(while creating the patch ...)

That strikes me as a huge restriction, and one that would surely be
mentioned prominently in the gmake manual if it were real. But I can't
find anything that says that.

I agree, and I don't think it's a problem which happens always.

I think what you are looking at here is a gmake bug, and that you should
report it to the gmake people.

Meybe we should ask them - but somehow I have the impression that
it's not a gmake problem ...

CU, Klaus

--
Klaus Naumann
Systems Administration
GMX Aktiengesellschaft
Riesstrasse 17, 80992 M�nchen
Telefon +49.89.143 39-0
Telefax +49.89.143 39-100
mailto:knaumann@gmx-ag.de
http://www.gmx.net/

Klaus Naumann <knaumann@gmx-ag.de> writes:

To me it looks like make is calling the target twice as there are
two dependent files.
I think this is due to a dependency of a dependency.
Lemme explain: What I think is, that we enter a directory and make
a parallel build. We want to compile prepare.c , so make calls the
rule to build prepare.c . A second make (because it's parallel)
sees a file which needs prepare.h, so it calls the rule.
At that point the rule is called twice. If it's now doing things
which are not supposed to work in parallel (like moving away
a file) the build fails.

Hmm. It sounds to me like the issue is not the dual-output rule as
such; it's that we have dependencies on one of the output files from
elsewhere in the source tree, and so we end up with multiple sub-makes
entering the backend/parser directory in parallel to build that header
file, and then it's broken because the rule is not safe for parallel
execution (quite independently of whether it has one or two output
files).

But that's supposed to be taken care of by having the parse.h file
updated before we start recursing into the rest of the backend --- see
src/backend/Makefile. Why is that not stopping the problem?

Did you see any actual failures related to the other bison invocations,
or just the one in backend/parser? AFAIK the other ones generate files
that are only used within their one source directory, and so they should
not have this problem anyway. There should only be one sub-make looking
at the other ones.

regards, tom lane

On Fri, 9 Nov 2001, Tom Lane wrote:

Klaus Naumann <knaumann@gmx-ag.de> writes:

To me it looks like make is calling the target twice as there are
two dependent files.
I think this is due to a dependency of a dependency.
Lemme explain: What I think is, that we enter a directory and make
a parallel build. We want to compile prepare.c , so make calls the
rule to build prepare.c . A second make (because it's parallel)
sees a file which needs prepare.h, so it calls the rule.
At that point the rule is called twice. If it's now doing things
which are not supposed to work in parallel (like moving away
a file) the build fails.

Hmm. It sounds to me like the issue is not the dual-output rule as
such; it's that we have dependencies on one of the output files from
elsewhere in the source tree, and so we end up with multiple sub-makes
entering the backend/parser directory in parallel to build that header
file, and then it's broken because the rule is not safe for parallel
execution (quite independently of whether it has one or two output
files).

This might be also true. But I that far to say, that this happens
within the same directory.
What I mean is, the file which generate the dependencies you are talking
about are in the same directory as the bison files.

But that's supposed to be taken care of by having the parse.h file
updated before we start recursing into the rest of the backend --- see
src/backend/Makefile. Why is that not stopping the problem?

s.a.

Did you see any actual failures related to the other bison invocations,
or just the one in backend/parser? AFAIK the other ones generate files
that are only used within their one source directory, and so they should
not have this problem anyway. There should only be one sub-make looking
at the other ones.

Yes, I also have seen races in two other directories
(and decided to change all occorences of the two-file
target in any Makefile because I'm under the impression
that it's to dangerous ...).
I have seen problems in src/backend/bootstrap and in
src/pl/plpgsql/src too.

Bye, Klaus

--
Klaus Naumann
Systems Administration
GMX Aktiengesellschaft
Riesstrasse 17, 80992 M�nchen
Telefon +49.89.143 39-0
Telefax +49.89.143 39-100
mailto:knaumann@gmx-ag.de
http://www.gmx.net/

Klaus Naumann <knaumann@gmx-ag.de> writes:

Yes, I also have seen races in two other directories
(and decided to change all occorences of the two-file
target in any Makefile because I'm under the impression
that it's to dangerous ...).

Now you're back to saying that any two-target rule is unsafe under
parallel make. I find that hard to believe, given the lack of any
suggestion in the gmake documents that any care needs to be taken
to make things safe for parallel makes.

regards, tom lane

On Fri, 9 Nov 2001, Tom Lane wrote:

Klaus Naumann <knaumann@gmx-ag.de> writes:

Yes, I also have seen races in two other directories
(and decided to change all occorences of the two-file
target in any Makefile because I'm under the impression
that it's to dangerous ...).

Now you're back to saying that any two-target rule is unsafe under
parallel make. I find that hard to believe, given the lack of any
suggestion in the gmake documents that any care needs to be taken
to make things safe for parallel makes.

At least the last sentence is just plain wrong. I have seen
and heard of several projects which had to change their
Makefiles to make parallel builds work.
A good example would be the linux kernel.
If you do a make -j3 dep clean bzImage you really f*ck up things
as it's all done in parallel. If you would add a rule

anything: dep clean bzImage

things would get messed up too (I have tried it ...).
I don't count this as a bug in make ...

And no, I'm not saying that any two-target rule is unsafe,
I just said I removed all as I think that it's a good idea
to aviod possible problems which might occur in the future.

CU, Klaus

--
Klaus Naumann
Systems Administration
GMX Aktiengesellschaft
Riesstrasse 17, 80992 M�nchen
Telefon +49.89.143 39-0
Telefax +49.89.143 39-100
mailto:knaumann@gmx-ag.de
http://www.gmx.net/

Tom Lane writes:

What bothers me is that you seem to be saying that *any* construct
involving multiple outputs from one rule is unsafe in a parallel make.
That strikes me as a huge restriction, and one that would surely be
mentioned prominently in the gmake manual if it were real. But I can't
find anything that says that.

I think we have been victims of a misunderstanding of how multiple-target
rules work. The gmake manual says:

"A rule with multiple targets is equivalent to writing many rules, each
with one target, and all identical aside from that."

So when we write this:

| $(srcdir)/preproc.c $(srcdir)/preproc.h: preproc.y
| $(YACC) -d $(YFLAGS) $<
| mv y.tab.c $(srcdir)/preproc.c
| mv y.tab.h $(srcdir)/preproc.h

make reads it as this:

| $(srcdir)/preproc.c: preproc.y
| $(YACC) -d $(YFLAGS) $<
| mv y.tab.c $(srcdir)/preproc.c
| mv y.tab.h $(srcdir)/preproc.h
|
| $(srcdir)/preproc.h: preproc.y
| $(YACC) -d $(YFLAGS) $<
| mv y.tab.c $(srcdir)/preproc.c
| mv y.tab.h $(srcdir)/preproc.h

So it's completely conceivable that both rules get executed in parallel.
(In general, you cannot optimize this away, because the actual commands
may differ depending on the value of $@.)

This basically means that you cannot correctly write a target that
generates more than one file. The only workaround I see is to cheat about
the dependencies, as Klaus' patch does:

y.tab.c: preproc.y
$(YACC) -d $(YFLAGS) $<

$(srcdir)/preproc.c: y.tab.c
mv y.tab.c $(srcdir)/preproc.c

$(srcdir)/preproc.h: y.tab.c
mv y.tab.h $(srcdir)/preproc.h

It's the latter rule where we're technically cheating and which I was
initially regarding as part of the "details I don't like", but now I think
it's the best (only?) way to go.

--
Peter Eisentraut peter_e@gmx.net

Peter Eisentraut <peter_e@gmx.net> writes:

y.tab.c: preproc.y
$(YACC) -d $(YFLAGS) $<

$(srcdir)/preproc.c: y.tab.c
mv y.tab.c $(srcdir)/preproc.c

$(srcdir)/preproc.h: y.tab.c
mv y.tab.h $(srcdir)/preproc.h

If you're going to do that, hadn't those last two better be "cp" not
"mv"?

What I find particularly distressing about this is that it will mean
that y.tab.c and y.tab.h have to become part of the shipped distribution
tarball. That's way ugly. I'd personally sooner document the existing
restriction: don't use parallel make on CVS sources.

As long as cheating is the order of the day, have you thought about

| $(srcdir)/preproc.c: preproc.y
| $(YACC) -d $(YFLAGS) $<
| mv y.tab.c $(srcdir)/preproc.c
| mv y.tab.h $(srcdir)/preproc.h
|
| $(srcdir)/preproc.h: $(srcdir)/preproc.c

Also, I'm still feeling that we are missing something fundamental about
parallel make. Surely there's got to be *some* interlock in make that
prevents multiple subjobs from executing the same rule in parallel.
Otherwise there are just too many things that are risky. I'm not
even convinced that "cp tmpfile targetfile" is totally safe under
such conditions.

regards, tom lane

I wrote:

Also, I'm still feeling that we are missing something fundamental about
parallel make. Surely there's got to be *some* interlock in make that
prevents multiple subjobs from executing the same rule in parallel.

Sigh, I guess my expectations are too high. Some digging around in
gmake 3.79.1 shows that the submakes coordinate only to the extent
of limiting the *number* of simultaneous child processes, not to the
extent of determining what the children are doing.

I don't think that there is any really bulletproof way to invoke bison
under this sort of scenario: if there are multiple submakes executing
the same build rule then it's entirely likely that we'll see things like
one child installing a y.tab.h file that's been truncated and only
partially rewritten by another child. Yes, the second child will
eventually make it good, but that's little comfort if the first child
launches compiles that fail meanwhile.

But we could provide some security for multiple children of a single
make by changing the rules to be like

$(srcdir)/parse.h: gram.y
ifdef YACC
$(YACC) -d $(YFLAGS) $<
mv y.tab.h $(srcdir)/parse.h
mv y.tab.c $(srcdir)/gram.c
else
@$(missing) bison $< $@
endif

$(srcdir)/gram.c: $(srcdir)/parse.h

Comments?

regards, tom lane

Tom Lane writes:

But we could provide some security for multiple children of a single
make by changing the rules to be like

$(srcdir)/parse.h: gram.y
ifdef YACC
$(YACC) -d $(YFLAGS) $<
mv y.tab.h $(srcdir)/parse.h
mv y.tab.c $(srcdir)/gram.c
else
@$(missing) bison $< $@
endif

$(srcdir)/gram.c: $(srcdir)/parse.h

That seems to be okay, although I think I'd switch gram.c and parse.h for
stylistic reasons.

--
Peter Eisentraut peter_e@gmx.net

Tom Lane writes:

Also, I'm still feeling that we are missing something fundamental about
parallel make. Surely there's got to be *some* interlock in make that
prevents multiple subjobs from executing the same rule in parallel.

There is, but remember that the two-target rule we're looking at is
actually two separate rules written in abbreviated form. And under that
pretext, the two rules are lying: They don't generate exactly the file
they have as the target. Unfortunately, there isn't a right way to fix
that.

Keep in mind that parallel make only executes the rule commands in
parallel, it does not cause the dependency analysis to be distributed.
In order to satisfy a dependency graph you only need to process each node
once, and parallelism doesn't change that.

--
Peter Eisentraut peter_e@gmx.net