Hi, On 2022-09-07 11:08:27 -0400, Tom Lane wrote: > > I'll need to think about how best to fix this. In the meantime, I > > think the other 32-bit animals are probably not going to like this > > either :-( > > Yeah. The basic problem here is that we've greatly reduced the amount > of redundancy in chunk headers.
Even with the prior amount of redundancy it's quite scary. It's one thing if the only consequence is a bit of added overhead - but if we *don't* copy the tuple due to a false positive we're in trouble. Afaict the user would have some control over the memory contents and so an attacker could work on triggering this issue. MemoryContextContains() may be ok for an assertion or such, but relying on it for correctness seems a bad idea. I wonder if we can get away from needing these checks, without unnecessarily copying datums every time: If there is no finalfunc, we know that the tuple ought to be in curaggcontext or such, and we need to copy. If there is a finalfunc, they're typically going to return data from within the current memory context, but could legitimately also return part of the data from curaggcontext. Perhaps we could forbid that? Our docs already say the following for serialization funcs: The result of the deserialization function should simply be allocated in the current memory context, as unlike the combine function's result, it is not long-lived. Perhaps we could institute a similar rule for finalfuncs? The argument against that is that we can use arbitrary functions as finalfuncs currently. Perhaps we could treat taking an internal argument as opting into the requirement and default to copying otherwise? Greetings, Andres Freund