[Bug 217390] New: use after free in spufs_switch_log_poll

[Bug 217390] New: use after free in spufs_switch_log_poll

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=217390

            Bug ID: 217390
           Summary: use after free in spufs_switch_log_poll
           Product: Platform Specific/Hardware
           Version: 2.5
          Hardware: PPC-32
                OS: Linux
            Status: NEW
          Severity: high
          Priority: P3
         Component: PPC-32
          Assignee: platform_ppc-32@kernel-bugs.osdl.org
          Reporter: james010kim@gmail.com
        Regression: No

When T2(poll) and T3(release) run concurrently by file_operations calls,
use-after-free happens due to the mistake of proper lock.

(ALLOC)
https://elixir.bootlin.com/linux/latest/source/arch/powerpc/platforms/cell/spufs/file.c#L2298
T1. open
2298 static int spufs_switch_log_open(struct inode *inode, struct file *file) {
2300    struct spu_context *ctx = SPUFS_I(inode)->i_ctx;
2301    int rc;
…..
2312    ctx->switch_log = kmalloc(struct_size(ctx->switch_log, log,
2313                              SWITCH_LOG_BUFSIZE), GFP_KERNEL); //
ALLOC-site
….
2327 }

The spufs_switch_log_open malloced ctx->switch_log that globally.

T2. poll
2431 static __poll_t spufs_switch_log_poll(struct file *file, poll_table *wait)
2432 {
2433    struct inode *inode = file_inode(file);
2434    struct spu_context *ctx = SPUFS_I(inode)->i_ctx;
2435    __poll_t mask = 0;
2436    int rc;

2438    poll_wait(file, &ctx->switch_log->wait, wait); // delayed by ‘wait’
callback
2430 // ctx->switch_log can be free by T3. 
2440    rc = spu_acquire(ctx);
2441    if (rc)
2442            return rc;

2444    if (spufs_switch_log_used(ctx) > 0) // USE-site
2445            mask |= EPOLLIN;

2447    spu_release(ctx);

2449    return mask;
2450 }

static inline void poll_wait(struct file * filp, wait_queue_head_t *
wait_address, poll_table *p)
{
        if (p && p->_qproc && wait_address)
                p->_qproc(filp, wait_address, p);  // The callback makes delays
}

T3. release
https://elixir.bootlin.com/linux/latest/source/arch/powerpc/platforms/cell/spufs/file.c#L2329
2329 static int spufs_switch_log_release(struct inode *inode, struct file
*file)
2330 {
2331    struct spu_context *ctx = SPUFS_I(inode)->i_ctx;
2332    int rc;

2334    rc = spu_acquire(ctx);
2335    if (rc)
2336            return rc;

2338    kfree(ctx->switch_log);   // FREE-site
2339    ctx->switch_log = NULL;
2340    spu_release(ctx);

2342    return 0;
2343 }


Fix could maybe be something like:

--- a/arch/powerpc/platforms/cell/spufs/file.c
+++ b/arch/powerpc/platforms/cell/spufs/file.c
*************** static __poll_t spufs_switch_log_poll(struct file *fil
*** 2435,2443 ****
        __poll_t mask = 0;
        int rc;

        poll_wait(file, &ctx->switch_log->wait, wait);

-       rc = spu_acquire(ctx);
        if (rc)
                return rc;

--- 2435,2443 ----
        __poll_t mask = 0;
        int rc;

+       rc = spu_acquire(ctx);
        poll_wait(file, &ctx->switch_log->wait, wait);

        if (rc)
                return rc;

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 217390] use after free in spufs_switch_log_poll

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=217390

James Kim (james010kim@gmail.com) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|high                        |normal

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.