On Sat, Apr 08, 2023 at 04:24:35PM +0200, Matthias van de Meent wrote:
> Thanks a lot! I'll post the separation of record construction and
> write-out to xlog in a future thread for 17.
Thanks! Creating a new thread makes sense.
> One remaining question: Considering that the changes and checks
On Fri, 7 Apr 2023 at 08:05, Michael Paquier wrote:
>
> On Fri, Apr 07, 2023 at 08:59:22AM +0900, Michael Paquier wrote:
> > Okay, cool!
>
> Done this one with 8fcb32d.
Thanks a lot! I'll post the separation of record construction and
write-out to xlog in a future thread for 17.
One remaining
On Fri, Apr 07, 2023 at 08:59:22AM +0900, Michael Paquier wrote:
> Okay, cool!
Done this one with 8fcb32d.
--
Michael
signature.asc
Description: PGP signature
On Fri, Apr 07, 2023 at 01:50:00AM +0200, Matthias van de Meent wrote:
> Yes, that was a bad oversight, which would've shown up in tests on a system
> with an endianness that my computer doesn't have...
I don't think that we have many bigendian animals in the buildfarm,
either..
> That looks
On Fri, 7 Apr 2023, 01:35 Michael Paquier, wrote:
> On Fri, Apr 07, 2023 at 08:08:34AM +0900, Michael Paquier wrote:
> > So bumping mainrdata_len to uint64 is actually not entirely in line
> > with this code. Well, it will work because we'd still fail a couple
> > of lines down, but perhaps its
On Fri, Apr 07, 2023 at 08:08:34AM +0900, Michael Paquier wrote:
> So bumping mainrdata_len to uint64 is actually not entirely in line
> with this code. Well, it will work because we'd still fail a couple
> of lines down, but perhaps its readability should be improved so as
> we have an extra
On Thu, Apr 06, 2023 at 10:54:43AM +0900, Michael Paquier wrote:
> 0002 can also be done before 0001, so I'd like to get that part
> applied on HEAD before the feature freeze and close this thread. If
> there are any objections, please feel free..
I was doing a pre-commit review of the patch,
On Wed, Apr 05, 2023 at 04:35:37PM +0200, Matthias van de Meent wrote:
> I thought that the plan was to use int64 to skip checking for most
> overflows and to do a single check at the end in XLogRecordAssemble,
> so that the checking has minimal overhead in the performance-critical
> log record
On Tue, 28 Mar 2023 at 13:42, Michael Paquier wrote:
>
> On Mon, Dec 19, 2022 at 12:37:19PM +0100, Alvaro Herrera wrote:
> > I have created one in the January commitfest,
> > https://commitfest.postgresql.org/41/
> > and rebased the patch on current master. (I have not reviewed this.)
>
> I have
On Mon, Dec 19, 2022 at 12:37:19PM +0100, Alvaro Herrera wrote:
> I have created one in the January commitfest,
> https://commitfest.postgresql.org/41/
> and rebased the patch on current master. (I have not reviewed this.)
I have spent some time on that, and here are some comments with an
On 2022-Dec-02, Andres Freund wrote:
> Hi,
>
> On 2022-12-02 14:22:55 +0900, Michael Paquier wrote:
> > On Fri, Nov 04, 2022 at 09:52:39AM +0900, Ian Lawrence Barwick wrote:
> > > CommitFest 2022-11 is currently underway, so if you are interested
> > > in moving this patch forward, now would be
Hi,
On 2022-07-26 18:58:02 +0200, Matthias van de Meent wrote:
> - updated the MaxXLogRecordSize and XLogRecordLengthIsValid(len)
> macros (now in xlogrecord.h), with a max length of the somewhat
> arbitrary 1020MiB.
> This leaves room for approx. 4MiB of per-record allocation overhead
> before
Hi,
On 2022-12-02 14:22:55 +0900, Michael Paquier wrote:
> On Fri, Nov 04, 2022 at 09:52:39AM +0900, Ian Lawrence Barwick wrote:
> > CommitFest 2022-11 is currently underway, so if you are interested
> > in moving this patch forward, now would be a good time to update it.
>
> No replies after 4
On Fri, Nov 04, 2022 at 09:52:39AM +0900, Ian Lawrence Barwick wrote:
> CommitFest 2022-11 is currently underway, so if you are interested
> in moving this patch forward, now would be a good time to update it.
No replies after 4 weeks, so I have marked this entry as returned
with feedback. I am
2022年10月5日(水) 16:46 Michael Paquier :
>
> Hi Matthias,
>
> On Wed, Jul 27, 2022 at 02:07:05PM +0200, Matthias van de Meent wrote:
>
> My apologies for the time it took me to come back to this thread.
> > > + * To accommodate some overhead, hhis MaxXLogRecordSize value allows for
> > >
Hi Matthias,
On Wed, Jul 27, 2022 at 02:07:05PM +0200, Matthias van de Meent wrote:
My apologies for the time it took me to come back to this thread.
> > + * To accommodate some overhead, hhis MaxXLogRecordSize value allows for
> > s/hhis/this/.
>
> Will be included in the next update..
v8
On Wed, 27 Jul 2022 at 11:09, Michael Paquier wrote:
>
> On Tue, Jul 26, 2022 at 06:58:02PM +0200, Matthias van de Meent wrote:
> > - Retained the check in XLogRegisterData, so that we check against
> > integer overflows in the registerdata code instead of only an assert
> > in XLogRecordAssemble
On Tue, Jul 26, 2022 at 06:58:02PM +0200, Matthias van de Meent wrote:
> - Retained the check in XLogRegisterData, so that we check against
> integer overflows in the registerdata code instead of only an assert
> in XLogRecordAssemble where it might be too late.
Why? The record has not been
On Tue, 26 Jul 2022 at 09:20, Michael Paquier wrote:
>
> On Mon, Jul 25, 2022 at 02:12:05PM +0300, Heikki Linnakangas wrote:
> > Among the two problems to solve at hand, the parts where the APIs are
> > changed and made more robust with unsigned types and where block data
> > is not overflowed
On Mon, Jul 25, 2022 at 02:12:05PM +0300, Heikki Linnakangas wrote:
> The way this is written, it would change whenever we add/remove fields in
> DecodedBkpBlock, for example. That's fragile; if you added a field in a
> back-branch, you could accidentally make the new minor version unable to
>
On Wed, 13 Jul 2022 at 07:54, Michael Paquier wrote:
>
> On Mon, Jul 11, 2022 at 02:26:46PM +0200, Matthias van de Meent wrote:
> > Thanks for reviewing.
>
> I think that v6 is over-engineered because there should be no need to
> add a check in xlogreader.c as long as the origin of the problem is
On 13/07/2022 08:54, Michael Paquier wrote:
I think that v6 is over-engineered because there should be no need to
add a check in xlogreader.c as long as the origin of the problem is
blocked, no? And the origin here is when the record is assembled. At
least this is the cleanest solution for
Hi,
On 2022-07-15 11:25:54 +0200, Matthias van de Meent wrote:
> On Mon, 14 Mar 2022 at 18:14, Andres Freund wrote:
> >
> > A random thought I had while thinking about the size limits: We could use
> > the
> > low bits of the length and xl_prev to store XLR_SPECIAL_REL_UPDATE |
> >
On Mon, 14 Mar 2022 at 18:14, Andres Freund wrote:
>
> A random thought I had while thinking about the size limits: We could use the
> low bits of the length and xl_prev to store XLR_SPECIAL_REL_UPDATE |
> XLR_CHECK_CONSISTENCY and give rmgrs the full 8 bit of xl_info. Which would
> allow us to
On Mon, Jul 11, 2022 at 02:26:46PM +0200, Matthias van de Meent wrote:
> Thanks for reviewing.
I think that v6 is over-engineered because there should be no need to
add a check in xlogreader.c as long as the origin of the problem is
blocked, no? And the origin here is when the record is
On Fri, 8 Jul 2022 at 21:35, David Zhang wrote:
>
> Hi,
>
> I tried to apply this patch v5 to current master branch but it complains,
> "git apply --check
> v5-0001-Add-protections-in-xlog-record-APIs-against-large.patch
> error: patch failed: src/include/access/xloginsert.h:43
> error:
Hi,
I tried to apply this patch v5 to current master branch but it complains,
"git apply --check
v5-0001-Add-protections-in-xlog-record-APIs-against-large.patch
error: patch failed: src/include/access/xloginsert.h:43
error: src/include/access/xloginsert.h: patch does not apply"
then I checked
On Tue, 21 Jun 2022 at 03:45, Michael Paquier wrote:
> + /*
> +* Ensure that xlogreader.c can read the record by ensuring that the
> +* data section of the WAL record can be allocated.
> +*/
> + if (unlikely(!AllocSizeIsValid(total_len)))
> + XLogErrorDataLimitExceeded();
>
On Mon, Jun 20, 2022 at 11:01:51AM +0200, Matthias van de Meent wrote:
> On Mon, 20 Jun 2022 at 07:02, Michael Paquier wrote:
>> + if (unlikely(!AllocSizeIsValid(total_len)))
>> + XLogErrorDataLimitExceeded();
>> Rather than a single check at the end of XLogRecordAssemble(), you'd
>>
On Mon, 20 Jun 2022 at 07:02, Michael Paquier wrote:
>
> On Sat, Jun 11, 2022 at 09:25:48PM +0200, Matthias van de Meent wrote:
> > Did you initiate a new cluster or otherwise skip the invalid record
> > you generated when running the instance based on master? It seems to
> > me you're trying to
On Sat, Jun 11, 2022 at 09:25:48PM +0200, Matthias van de Meent wrote:
> Did you initiate a new cluster or otherwise skip the invalid record
> you generated when running the instance based on master? It seems to
> me you're trying to replay the invalid record (len > MaxAllocSize),
> and this patch
A little confused here, does this patch V3 intend to solve this problem "record
length 2145386550 at 0/360 too long"?
No, not once the record exists. But it does remove Postgres' ability
to create such records, thereby solving the problem for all systems
that generate WAL through
On Sat, 11 Jun 2022 at 01:32, David Zhang wrote:
>
> Hi,
>
> > > MaxAllocSize is pretty easy:
> > > SELECT pg_logical_emit_message(false, long, long) FROM repeat(repeat(' ',
> > > 1024), 1024*1023) as l(long);
> > >
> > > on a standby:
> > >
> > > 2022-03-11 16:41:59.336 PST
Hi,
> > MaxAllocSize is pretty easy:
> > SELECT pg_logical_emit_message(false, long, long) FROM
repeat(repeat(' ', 1024), 1024*1023) as l(long);
> >
> > on a standby:
> >
> > 2022-03-11 16:41:59.336 PST [3639744][startup][1/0:0] LOG: record
length 2145386550 at 0/360 too long
>
> Thanks
On Mon, Apr 18, 2022 at 05:48:50PM +0200, Matthias van de Meent wrote:
> Seeing that the busiest time for PG15 - the last commitfest before the
> feature freeze - has passed, could someone take another look at this?
The next minor release is three weeks away, so now would be a good
time to get
Seeing that the busiest time for PG15 - the last commitfest before the
feature freeze - has passed, could someone take another look at this?
The changes that were requested by Heikki and Andres have been merged
into patch v3, and I think it would be nice to fix this security issue
in the upcoming
Apart from registering this in CF 2022-07 last Friday, I've also just
added this issue to the Open Items list for PG15 under "Older bugs
affecting stable branches"; as a precaution to not lose track of this
issue in the buzz of the upcoming feature freeze.
-Matthias
On Mon, 14 Mar 2022 at 18:14, Andres Freund wrote:
>
> Hi
>
> A random thought I had while thinking about the size limits: We could use the
> low bits of the length and xl_prev to store XLR_SPECIAL_REL_UPDATE |
> XLR_CHECK_CONSISTENCY and give rmgrs the full 8 bit of xl_info. Which would
> allow
Hi
A random thought I had while thinking about the size limits: We could use the
low bits of the length and xl_prev to store XLR_SPECIAL_REL_UPDATE |
XLR_CHECK_CONSISTENCY and give rmgrs the full 8 bit of xl_info. Which would
allow us to e.g. get away from needing Heap2. Which would aestethically
Thank you all for the feedback. Please find attached v2 of the
patchset, which contains updated comments and applies the suggested
changes.
On Sat, 12 Mar 2022 at 02:03, Andres Freund wrote:
>
> Hi,
>
> On 2022-03-11 22:42:42 +0200, Heikki Linnakangas wrote:
> > Have you been able to create a
Hi,
On 2022-03-11 22:42:42 +0200, Heikki Linnakangas wrote:
> Have you been able to create a test case for that? The largest record I can
> think of is a commit record with a huge number of subtransactions, dropped
> relations, and shared inval messages. I'm not sure if you can overflow a
>
On Fri, Mar 11, 2022 at 3:42 PM Heikki Linnakangas wrote:
> Have you been able to create a test case for that? The largest record I
> can think of is a commit record with a huge number of subtransactions,
> dropped relations, and shared inval messages. I'm not sure if you can
> overflow a uint32
On 11/03/2022 17:42, Matthias van de Meent wrote:
Hi,
Xlogreader limits the size of what it considers valid xlog records to
MaxAllocSize; but this is not currently enforced in the
XLogRecAssemble API. This means it is possible to assemble a record
that postgresql cannot replay.
Oops, that
Hi,
Xlogreader limits the size of what it considers valid xlog records to
MaxAllocSize; but this is not currently enforced in the
XLogRecAssemble API. This means it is possible to assemble a record
that postgresql cannot replay.
Similarly; it is possible to repeatedly call XlogRegisterData() so
44 matches
Mail list logo
