Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 04 May 2023 10:28:56 +0200
Message-ID: <875y98mref.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     mchehab@kernel.org
Cc:     Yu Hao <yhao016@ucr.edu>, linux-media@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: BUG: WARNING in dvb_frontend_get_event
In-Reply-To: <87bkj9u57j.wl-tiwai@suse.de>
References: <CA+UBctCu7fXn4q41O_3=id1+OdyQ85tZY1x+TkT-6OVBL6KAUw@mail.gmail.com>
        <87bkj9u57j.wl-tiwai@suse.de>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

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;
>  	}