Skip to contentSkip to conversation

New: use after free in spufs_switch_log_poll

Bug 217390 [email protected] 1 participant 2 messages May 2, 2023

lore
Share & tools
lore
#0 lore raw

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: [email protected]
Reporter: [email protected]
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;
Signature · 4 lines
You may reply to this email to add a comment.

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