Bug 217390

Summary: use after free in spufs_switch_log_poll
Product: Platform Specific/Hardware Reporter: James Kim (james010kim)
Component: PPC-32Assignee: platform_ppc-32
Status: NEW ---    
Severity: normal CC: james010kim
Priority: P3    
Hardware: PPC-32   
OS: Linux   
Kernel Version: Subsystem:
Regression: No Bisected commit-id:

Description James Kim 2023-05-02 11:46:56 UTC
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;