Re: [PATCH 3/8] Add support for a generic wal reading facility dubbed XLogReader

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>
Subject: Re: [PATCH 3/8] Add support for a generic wal reading facility dubbed XLogReader
Date: 2012-10-30 15:32:31
Message-ID: 201210301632.33907.andres@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tuesday, October 30, 2012 04:24:21 PM Alvaro Herrera wrote:
> Tom Lane escribió:
> > Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > > On Tuesday, October 30, 2012 03:20:03 PM Alvaro Herrera wrote:
> > >> Am I the only one who finds this rather bizarre? Maybe this was okay
> > >> when xlog data would only come from WAL files stored in the data
> > >> directory at recovery, but if we're now receiving these from a remote
> > >> sender over the network I wonder if we should be protecting against
> > >> malicious senders. (This is not related to this patch anyway.)
> > >
> > > You can't use a CRC against malicous users anyway, its not
> > > cryptographically secure in any meaning of the word,
> >
> > More to the point, if a bad guy has got control of your WAL stream,
> > crashing the startup process with a ridiculous length word is several
> > orders of magnitude less than the worst thing he can find to do to you.
> > So the above argument seems completely nonsensical to me.
>
> Well, I wasn't talking just about the record length, but about the
> record in general. The length just came up because it's what I noticed.
>
> And yeah, I was thinking in one sum for the header and another one for
> the data. If we're using CRC to detect end of WAL, what sense does it
> make to have to read the whole record if we can detect the end by just
> looking at the header? (I obviously see that we need to checksum the
> data as well; and having a second CRC field would bloat the record.)

Well, the header is written first. In the header we can detect somewhat
accurately that were beyond the current end-of-wal by looking at ->xl_prev and
doing some validity checks, but thats not applicable for the data. A valid
looking header doesn't mean that the whole, potentially multi-megabyte, record
data is valid, we could have crashed while writing the data.

Greetings,

Andres
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Christian Kruse 2012-10-30 19:16:33 Re: Re: [HACKERS] Patch für MAP_HUGETLB for mmap() shared memory
Previous Message Tom Lane 2012-10-30 15:30:43 Re: [PATCH 3/8] Add support for a generic wal reading facility dubbed XLogReader