talloc_tos()

[Jeremy Allison]

On Fri, Jun 15, 2018 at 06:18:41PM +0200, David Disseldorp wrote:
> On Fri, 15 Jun 2018 08:44:16 -0700, Jeremy Allison wrote:
> 
> > > A convention of using talloc_tos() does not guarantee that we don't
> > > leak memory. Just like long running async state contexts, talloc
> > > stackframes can also be neglected. As mentioned in the other mail, my
> > > preference would be to proceed with integration of valgrind memcheck,
> > > massif and talloc_report into regular CI.  
> > 
> > I respect your opinion, but talloc_tos() was created to fix
> > specific errors that commonly crept into the code whilst
> > we were trying to do *precisely* what you're recommending :-).
> 
> FWIW, I do realise that my proposal of a core change like talloc_tos()
> removal may come across as a little condescending. If that's the case,
> I apologise to Volker and yourself.
> 
> > In short, been there - tried that, it's not possible with
> > flawed humans to have the discipline needed to take care
> > of the talloc contexts correctly. I remember debugging these
> > problems, it was not fun.
> 
> Hopefully tools like Valgrind and AddressSanitizer have improved since
> those days :)

No, they really haven't, and they can't. The problem we face is that these
tools don't address this problem *AT ALL*. That's simply not what
they are for.

The issue is a developer typo can mean short-term memory leaks onto
a long term context.

There's no address misuse, there's no memory corruption,
no actual *bug* there for these tools to find. They can't
know the mind of the developer and what is the difference
between a short and long term context.

These bugs are the *worst* to find, and destroy confidence
in the project as a whole as they bring down unrelated
processes on the box, and gain you a reputation of incompetence
and unreliability. Gnome-shell is an excellent example of this.

Crashes are simple to fix and sweet by comparison :-).

> > Does talloc_tos() fix all that ? No, flawed humans screw
> > that up too. But I've used both mechanisms, and we have
> > *much* less bugs with talloc_tos().
> > 
> > tl;dr - talloc_tos() misuse crashes are much preferable to
> > long-lived memory leaks by trying to always use the correct
> > talloc context.
> 
> talloc_tos() misuse use-after-free bugs, as opposed to long-lived leaks,
> have more serious security implications. I'm trying to avoid sounding
> like a broken record, but I really do believe any benefits are
> outweighed by the drawbacks, especially when going event based.

Yeah, and I utterly disagree :-).

Most of the use-after-free bugs are not security issues,
other than a (usually authenticated) crash of smbd DOS.

But we do have a control experiment I proposed in my
other message. Count the leaks...

Jeremy.