search

bingghost / google-security-research

Automatically exported from code.google.com/p/google-security-research
0 stars 0 forks source link

Samsung fimg2d FIMG2D_BITBLT_BLIT ioctl concurrency flaw #492

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago 
The Samsung Graphics 2D driver (/dev/fimg2d) is accessible by unprivileged 
users/applications. It was found that the ioctl implementation for this driver 
contains a locking error which can lead to memory errors (such as 
use-after-free) due to a race condition.

The key observation is in the locking routine definitions in fimg2d.h:

#ifdef BLIT_WORKQUE
#define g2d_lock(x)             do {} while (0)
#define g2d_unlock(x)           do {} while (0)
#define g2d_spin_lock(x, f)     spin_lock_irqsave(x, f)
#define g2d_spin_unlock(x, f)   spin_unlock_irqrestore(x, f)
#else
#define g2d_lock(x)             mutex_lock(x)
#define g2d_unlock(x)           mutex_unlock(x)
#define g2d_spin_lock(x, f)     do { f = 0; } while (0)
#define g2d_spin_unlock(x, f)   do { f = 0; } while (0)
#endif

This means that the g2d_lock/g2d_unlock routines are no-ops when BLIT_WORKQUE 
is defined, which appears to be the default configuration. Unfortunately the 
alternative spin lock routines are not used consistently with this 
configuration. For example, the FIMG2D_BITBLT_BLIT ioctl command (with notes 
annotated as "PZ"):

ctx = file->private_data; /* PZ: ctx allocated at open(), lives on the heap. */

switch (cmd) {
case FIMG2D_BITBLT_BLIT:

    mm = get_task_mm(current);
    if (!mm) {
        fimg2d_err("no mm for ctx\n");
        return -ENXIO;
    }

    g2d_lock(&ctrl->drvlock); /* PZ: This is a no-op. */

    ctx->mm = mm;

    ret = fimg2d_add_command(ctrl, ctx, (struct fimg2d_blit __user *)arg);
    if (ret) {
        ...
    }

    ret = fimg2d_request_bitblt(ctrl, ctx); /* PZ: Does stuff with the ctx. */
    if (ret) {
        ...
    }

    g2d_unlock(&ctrl->drvlock); /* PZ: Another no-op */

As the lock macros are no-ops, a second process can change ctx->mm when the 
original process is still using the same ctx->mm (as long as it has access to 
the same file descriptor).

Reproduction steps:
Open /dev/fimg2d
Fork to get two processes with different mm’s with the access to the fd
Concurrently call the FIMG2D_BITBLT_BLIT ioctl from both processes.
One ioctl should have valid data, the other should fail

At this point ctx->mm will now have invalid or free data (free if the forked 
process dies). Proof-of-concept code to trigger this condition is attached 
(fimg2d-lock.c)

This bug is subject to a 90 day disclosure deadline. If 90 days elapse
without a broadly available patch, then the bug report will automatically
become visible to the public.

Original issue reported on code.google.com by haw...@google.com on 29 Jul 2015 at 10:05

Attachments:

GoogleCodeExporter commented 8 years ago

Original comment by scvi...@google.com on 30 Jul 2015 at 2:11

GoogleCodeExporter commented 8 years ago

Original comment by natashe...@google.com on 23 Oct 2015 at 6:17

GoogleCodeExporter commented 8 years ago 
Fixed in October MR.

Original comment by natashe...@google.com on 27 Oct 2015 at 6:39

GoogleCodeExporter commented 8 years ago

Original comment by natashe...@google.com on 27 Oct 2015 at 6:40