NULL ptr dereference in i915_gem_alloc_object()

[Daniel Vetter]

On Sun, Jan 19, 2014 at 8:39 PM, Linus Torvalds
<torvalds at linux-foundation.org> wrote:
> On Sun, Jan 19, 2014 at 9:31 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
>>
>> This took much longer than I'll ever dare to admit, but I think I've
>> stitched a testcase together for this and pushed it to our i915 test repo:
>
> So I was testing a patch that limits one user to fewer than the global
> system resource files, and the oops happened just once, so it might
> have been a lucky fluke, with the failure *usually* happening
> somewhere else.
>
> And I'm beginning to think I misread the oops. I was *assuming* it was
> due to a NULL file pointer access in that function because of the way
> I triggered it, but now that I look closer at the disassembly, what
> seems to actually be happening is that the (inlined)
> i915_gem_object_free() function does
>
>         struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
>         kmem_cache_free(dev_priv->slab, obj);
>
> and it's "base.dev" that is NULL, so the dev_private access is the one
> that oopses.
>
> And it is related to running out of file descriptors, but indirectly:
> the lack of file descriptors is making drm_gem_object_init() fail
> (correctly), but it has failed *before* it has done the
>
>         drm_gem_private_object_init(dev, obj, size);
>
> which is why that oops then happens.
>
> So my suggestion is that drm_gem_object_init() should do that
> drm_gem_private_object_init() part regardless of whether the
> shm_file_setup() failed or not.
>
> Makes sense?

Yup. And I've also figured out why my testcase couldn't kill the
kernel - the testsuite is run as root to be able to do generally nasty
things, and apparently the kernel doesn't enforce the limits for root.
Now that I drop root privs in the testcase it blows up in a nice oops.
And is fixed with your suggestion applied.

But then my testcase now dies up on an assert where it shouldn't, so I
think I'll look at all this again tomorrow with a fresh mind, clean up
the patches and then send them out.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch