At Thu, 22 Feb 2018 16:22:47 -0800, Peter Geoghegan <p...@bowt.ie> wrote in <CAH2-Wz=QW6FNpLEfQpFKmKiu_WkfxCpmPDmp6ZUf=7srars...@mail.gmail.com> > On Mon, Feb 12, 2018 at 6:15 PM, Peter Geoghegan <p...@bowt.ie> wrote: > > * For 9.5 and 9.6, the approach taken in bugfix commit d8589946d > > should be taken even further -- we should always copy. Moreover, we > > should always copy within tuplesort_getdatum(), for the same reasons. > > > > * For 9.5, 9.6, 10, and master, we should make sure that > > tuplesort_getdatum() uses the caller's memory context. The fact that > > it doesn't already do so seems like a simple oversight. We should do > > this to be consistent with tuplesort_gettupleslot(). (This isn't > > critical, but seems like a good idea.) > > > > * For 9.5, 9.6, 10, and master, we should adjust some comments from > > tuplesort_getdatum() callers, so that they no longer say that > > tuplesort datum tuple memory lives in tuplesort context. That won't be > > true anymore. > > Attached patches do it that way. I'm happy with what I came up with, > which is a lot simpler than my first approach. The extra copying seems > likely to be well worth it, since it is fairly isolated in practice, > especially on 9.6. There is no extra copying from v10+, since they > don't need the first fix at all.
FWIW I examined the case by myself. And I confirmed that the cause is tuple freeing after tuplesort_end conducted by shouldFree. As a principle, since it is declared that the caller is responsible to free the result, tuplesort shouldn't free it out of the caller's control. Even if it is not currently causig use-after-free problem, it is also possible to happen. >From this point of view, the patch for 9.5 and 9.6 looks fine and actually fixes the problem. For 10+, copying is controlled by the caller side, but only tuplesort_getdatum() didn't make the copy in the caller context. It is just an overlook and the fix looks reasonable. I'm not appropriate for checking comment wording but it makes sense for me. If no one objects, I'll mark this as Ready for Commit in a couple of days. reagards, -- Kyotaro Horiguchi NTT Open Source Software Center
