Re: BUG: WARNING in dvb_frontend_get_event

[Takashi Iwai]

On Thu, 27 Apr 2023 09:58:24 +0200,
Takashi Iwai wrote:
> 
> On Tue, 18 Apr 2023 06:50:07 +0200,
> Yu Hao wrote:
> > 
> > Hello,
> > 
> > We found the following issue using syzkaller on Linux v6.2.0.
> > 
> > In the function `dvb_frontend_get_event`, function
> > `wait_event_interruptible` is called
> > and the condition is `dvb_frontend_test_event(fepriv, events)`.
> > In the function `dvb_frontend_test_event`, function
> > `down(&fepriv->sem);` is called.
> > However, function `wait_event_interruptible` would put the process to sleep.
> > And function `down(&fepriv->sem);` may block the process.
> > So there is the issue with "do not call blocking ops when !TASK_RUNNING".
> > 
> > The full report including the Syzkaller reproducer & C reproducer:
> > https://gist.github.com/ZHYfeng/4c5f8be6adc63b73dba68230d15ece2c
> 
> FYI, CVE-2023-31084 was assigned to this bug, and I was involved now
> though distro's bug report.
> 
> So, the use of semaphore together with wait_event*() macro doesn't
> look like a good idea.  A possible easy workaround would be to
> open-code the wait loop like below.
> 
> Mauro, let me know if it's an acceptable workaround.  Then I'll submit
> a proper patch.

A gentle ping.

Can anyone please check whether the suggested change is OK or not?


thanks,

Takashi

> 
> 
> thanks,
> 
> Takashi
> 
> -- 8< --
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -293,14 +293,22 @@ static int dvb_frontend_get_event(struct dvb_frontend *fe,
>  	}
>  
>  	if (events->eventw == events->eventr) {
> -		int ret;
> +		struct wait_queue_entry wait;
> +		int ret = 0;
>  
>  		if (flags & O_NONBLOCK)
>  			return -EWOULDBLOCK;
>  
> -		ret = wait_event_interruptible(events->wait_queue,
> -					       dvb_frontend_test_event(fepriv, events));
> -
> +		init_waitqueue_entry(&wait, current);
> +		add_wait_queue(&events->wait_queue, &wait);
> +		while (!dvb_frontend_test_event(fepriv, events)) {
> +			wait_woken(&wait, TASK_INTERRUPTIBLE, 0);
> +			if (signal_pending(current)) {
> +				ret = -ERESTARTSYS;
> +				break;
> +			}
> +		}
> +		remove_wait_queue(&events->wait_queue, &wait);
>  		if (ret < 0)
>  			return ret;
>  	}