[Development] QtCore missing check for memory allocation

[Thiago Macieira]

On Wednesday 25 February 2015 16:48:44 Ulf Hermann wrote:
> >> We should thus do Q_CHECK_PTR on every memory allocation in Qt and we
> >> should fix Q_CHECK_PTR so that it works under all circumstances.
> > 
> > I disagree on both accounts.
> 
> Could you elaborate a bit?

I disagree that we should do Q_CHECK_PTR after every memory allocation and I 
disagree that Q_CHECK_PTR should do something in all circumstances.

> That "every memory allocation" may be relaxed a bit as there might be places
> where the code can deal with 0 or where we pass the pointer straight on to
> user code and can expect the user to check it. The default should be
> checking for 0, though.

I really disagree. If we wanted to find bad memory allocations, we should turn 
exceptions back on and write exception-safe code. Anything else is putting the 
burden on the common code path.

> > That's undefined behaviour. If you write code:
> > 	if (!p)
> > 	
> > 		*(char*)p = 42;		// crash
> 
> I'm not saying we should access p in this case, but rather some fixed place
> of which we know we cannot access it, to *reliably* raise a segfault. Maybe
> there is even a more elegant way to trigger a segfault than accessing
> invalid memory.

The only reliable way of causing a segfault is raise(SIGSEGV). You can't 
reliably trigger a memory problem because, by the very definition of it, the 
compiler is allowed to assume it doesn't happen.

> The point is this: With the current behavior you're not actually guaranteed
> to get a segfault. The client code might not access *p, but rather p[<some
> large number>], and that might hit valid memory. Or it might store p, do
> whatever funny arithmetic on it with the assumption that it's not 0, and
> run into some other incorrect behavior. An attacker could specifically
> search for such a case and overwrite some important piece of information
> like this. We don't want that. Q_CHECK_PTR should mean "If the pointer is 0
> either throw an exception or abort right away. Don't just continue."

I understand your arguments, but I still disagree we should act.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center