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, 27 Apr 2023 09:58:24 +0200
Message-ID: <87bkj9u57j.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     Yu Hao <yhao016@ucr.edu>
Cc:     mchehab@kernel.org, linux-media@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: BUG: WARNING in dvb_frontend_get_event
In-Reply-To: <CA+UBctCu7fXn4q41O_3=id1+OdyQ85tZY1x+TkT-6OVBL6KAUw@mail.gmail.com>
References: <CA+UBctCu7fXn4q41O_3=id1+OdyQ85tZY1x+TkT-6OVBL6KAUw@mail.gmail.com>
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 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.


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