mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 02:30:34 +02:00 
			
		
		
		
	media: vivid: Fix wrong locking that causes race conditions on streaming stop
There is the same incorrect approach to locking implemented in vivid_stop_generating_vid_cap(), vivid_stop_generating_vid_out() and sdr_cap_stop_streaming(). These functions are called during streaming stopping with vivid_dev.mutex locked. And they all do the same mistake while stopping their kthreads, which need to lock this mutex as well. See the example from vivid_stop_generating_vid_cap(): /* shutdown control thread */ vivid_grab_controls(dev, false); mutex_unlock(&dev->mutex); kthread_stop(dev->kthread_vid_cap); dev->kthread_vid_cap = NULL; mutex_lock(&dev->mutex); But when this mutex is unlocked, another vb2_fop_read() can lock it instead of vivid_thread_vid_cap() and manipulate the buffer queue. That causes a use-after-free access later. To fix those issues let's: 1. avoid unlocking the mutex in vivid_stop_generating_vid_cap(), vivid_stop_generating_vid_out() and sdr_cap_stop_streaming(); 2. use mutex_trylock() with schedule_timeout_uninterruptible() in the loops of the vivid kthread handlers. Signed-off-by: Alexander Popov <alex.popov@linux.com> Acked-by: Linus Torvalds <torvalds@linux-foundation.org> Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Cc: <stable@vger.kernel.org> # for v3.18 and up Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
This commit is contained in:
		
							parent
							
								
									0c90f649d2
								
							
						
					
					
						commit
						6dcd5d7a7a
					
				
					 3 changed files with 15 additions and 9 deletions
				
			
		| 
						 | 
					@ -818,7 +818,11 @@ static int vivid_thread_vid_cap(void *data)
 | 
				
			||||||
		if (kthread_should_stop())
 | 
							if (kthread_should_stop())
 | 
				
			||||||
			break;
 | 
								break;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		mutex_lock(&dev->mutex);
 | 
							if (!mutex_trylock(&dev->mutex)) {
 | 
				
			||||||
 | 
								schedule_timeout_uninterruptible(1);
 | 
				
			||||||
 | 
								continue;
 | 
				
			||||||
 | 
							}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		cur_jiffies = jiffies;
 | 
							cur_jiffies = jiffies;
 | 
				
			||||||
		if (dev->cap_seq_resync) {
 | 
							if (dev->cap_seq_resync) {
 | 
				
			||||||
			dev->jiffies_vid_cap = cur_jiffies;
 | 
								dev->jiffies_vid_cap = cur_jiffies;
 | 
				
			||||||
| 
						 | 
					@ -998,8 +1002,6 @@ void vivid_stop_generating_vid_cap(struct vivid_dev *dev, bool *pstreaming)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	/* shutdown control thread */
 | 
						/* shutdown control thread */
 | 
				
			||||||
	vivid_grab_controls(dev, false);
 | 
						vivid_grab_controls(dev, false);
 | 
				
			||||||
	mutex_unlock(&dev->mutex);
 | 
					 | 
				
			||||||
	kthread_stop(dev->kthread_vid_cap);
 | 
						kthread_stop(dev->kthread_vid_cap);
 | 
				
			||||||
	dev->kthread_vid_cap = NULL;
 | 
						dev->kthread_vid_cap = NULL;
 | 
				
			||||||
	mutex_lock(&dev->mutex);
 | 
					 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -166,7 +166,11 @@ static int vivid_thread_vid_out(void *data)
 | 
				
			||||||
		if (kthread_should_stop())
 | 
							if (kthread_should_stop())
 | 
				
			||||||
			break;
 | 
								break;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		mutex_lock(&dev->mutex);
 | 
							if (!mutex_trylock(&dev->mutex)) {
 | 
				
			||||||
 | 
								schedule_timeout_uninterruptible(1);
 | 
				
			||||||
 | 
								continue;
 | 
				
			||||||
 | 
							}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		cur_jiffies = jiffies;
 | 
							cur_jiffies = jiffies;
 | 
				
			||||||
		if (dev->out_seq_resync) {
 | 
							if (dev->out_seq_resync) {
 | 
				
			||||||
			dev->jiffies_vid_out = cur_jiffies;
 | 
								dev->jiffies_vid_out = cur_jiffies;
 | 
				
			||||||
| 
						 | 
					@ -344,8 +348,6 @@ void vivid_stop_generating_vid_out(struct vivid_dev *dev, bool *pstreaming)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	/* shutdown control thread */
 | 
						/* shutdown control thread */
 | 
				
			||||||
	vivid_grab_controls(dev, false);
 | 
						vivid_grab_controls(dev, false);
 | 
				
			||||||
	mutex_unlock(&dev->mutex);
 | 
					 | 
				
			||||||
	kthread_stop(dev->kthread_vid_out);
 | 
						kthread_stop(dev->kthread_vid_out);
 | 
				
			||||||
	dev->kthread_vid_out = NULL;
 | 
						dev->kthread_vid_out = NULL;
 | 
				
			||||||
	mutex_lock(&dev->mutex);
 | 
					 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -141,7 +141,11 @@ static int vivid_thread_sdr_cap(void *data)
 | 
				
			||||||
		if (kthread_should_stop())
 | 
							if (kthread_should_stop())
 | 
				
			||||||
			break;
 | 
								break;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		mutex_lock(&dev->mutex);
 | 
							if (!mutex_trylock(&dev->mutex)) {
 | 
				
			||||||
 | 
								schedule_timeout_uninterruptible(1);
 | 
				
			||||||
 | 
								continue;
 | 
				
			||||||
 | 
							}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		cur_jiffies = jiffies;
 | 
							cur_jiffies = jiffies;
 | 
				
			||||||
		if (dev->sdr_cap_seq_resync) {
 | 
							if (dev->sdr_cap_seq_resync) {
 | 
				
			||||||
			dev->jiffies_sdr_cap = cur_jiffies;
 | 
								dev->jiffies_sdr_cap = cur_jiffies;
 | 
				
			||||||
| 
						 | 
					@ -303,10 +307,8 @@ static void sdr_cap_stop_streaming(struct vb2_queue *vq)
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	/* shutdown control thread */
 | 
						/* shutdown control thread */
 | 
				
			||||||
	mutex_unlock(&dev->mutex);
 | 
					 | 
				
			||||||
	kthread_stop(dev->kthread_sdr_cap);
 | 
						kthread_stop(dev->kthread_sdr_cap);
 | 
				
			||||||
	dev->kthread_sdr_cap = NULL;
 | 
						dev->kthread_sdr_cap = NULL;
 | 
				
			||||||
	mutex_lock(&dev->mutex);
 | 
					 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static void sdr_cap_buf_request_complete(struct vb2_buffer *vb)
 | 
					static void sdr_cap_buf_request_complete(struct vb2_buffer *vb)
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue