Re: A modest proposal: make parser/rewriter/planner inputs read-only

On Sat, Apr 5, 2025 at 7:31 PM Andres Freund <andres(at)anarazel(dot)de> wrote:

> Hi,
>
> On 2025-04-05 12:46:37 -0400, Tom Lane wrote:
> > 1. Invent a way to make a particular memory context read-only
> > after putting some data into it.
> >
> > 2. In debug builds, after we've built a tree that should be considered
> > read-only, copy it into such a context and make it read-only. Or
> > perhaps build it there in the first place.
>
> > 3. Fix the resulting crashes.
> >
> > 4. Profit! (In particular, nuke a lot of no-longer-needed copyObject
> > calls.)
> >
> > My first thought about implementing #1 was to seek Valgrind's help,
> > but so far as I can find out there's no VALGRIND_MAKE_MEM_READ_ONLY.
> > Step #3 would be pretty tedious anyway if it required running under
> > Valgrind. However, all modern hardware has the ability to mark
> > memory read-only at the page level, and most platforms expose that
> > in some way or other. So it doesn't seem unreasonable to invent
> > a memory context option (or whole new context type, if that seems
> > easier) that is careful to align its allocation blocks on page
> > boundaries and then can set or clear the hardware R/O flag on
> > demand. It'd be enough if the R/O enforcement worked on popular
> > development platforms, we don't have to make it work absolutely
> > everywhere.
>
> FWIW, while hacking on patch to making hint bit writes not happening while
> IO
> is going on (so we don't need to copy the page anymore and don't cause
> filesystem level issues with DIO), I hacked up protection for shared
> buffers
> using mprotect() - it worked way better than I thought it would. The
> overhead
> ended up surprisingly low:
>
> base:
> real 1m4.613s
> user 4m31.409s
> sys 3m20.445s
>
> ENFORCE_BUFFER_PROT
>
> real 1m11.912s
> user 4m27.332s
> sys 3m28.063s
>
>
> See https://postgr.es/m/043c8b50-d183-46e5-b054-145cc0f6f908%40iki.fi
>
>
> I'm mostly sharing that to say that
> a) yes, mprotect() is viable and works surprisingly well
> b) it might be worth inventing some common platform abstraction for
> mprotect
>
> That prototype patch already worked on most platforms, windows should be
> entirely doable.

Also, I would like to provide reference to this old thread [1] in favor of
mprotect().
In that thread, and in Greenplum using mprotect helps detect Shared buffer
access rule violations.

[1]
https://www.postgresql.org/message-id/CANXE4Tfxmjv8Z-kcwWUCYxx6zCFS%3DGeBPm-ZjjLucvrGewjUtg%40mail.gmail.com

--
This electronic communication and the information and any files transmitted
with it, or attached to it, are confidential and are intended solely for
the use of the individual or entity to whom it is addressed and may contain
information that is confidential, legally privileged, protected by privacy
laws, or otherwise restricted from disclosure to anyone else. If you are
not the intended recipient or the person responsible for delivering the
e-mail to the intended recipient, you are hereby notified that any use,
copying, distributing, dissemination, forwarding, printing, or copying of
this e-mail is strictly prohibited. If you received this e-mail in error,
please return the e-mail to the sender, delete it from your computer, and
destroy any printed copy of it.