On Sun, 2018-07-01 at 18:02 +0200, Greg Kroah-Hartman wrote:
4.4-stable review patch. If anyone has any objections, please let me know.
From: Mauro Carvalho Chehab mchehab@s-opensource.com
commit 76d81243a487c09619822ef8e7201a756e58a87d upstream.
As warned by smatch: drivers/media/dvb-core/dvb_frontend.c:314 dvb_frontend_get_event() warn: inconsistent returns 'sem:&fepriv->sem'. Locked on: line 288 line 295 line 306 line 314 Unlocked on: line 303
The lock implementation for get event is wrong, as, if an interrupt occurs, down_interruptible() will fail, and the routine will call up() twice when userspace calls the ioctl again.
The bad code is there since when Linux migrated to git, in 2005.
[...]
--- a/drivers/media/dvb-core/dvb_frontend.c +++ b/drivers/media/dvb-core/dvb_frontend.c
[...]
+static int dvb_frontend_test_event(struct dvb_frontend_private *fepriv,
struct dvb_fe_events *events)+{
- int ret;
- up(&fepriv->sem);
- ret = events->eventw != events->eventr;
- down(&fepriv->sem);
- return ret;
+}
This is broken. You must not wait inside a wait condition. (It would be nice if this rule was explicitly documented.)
Why not leave the condition as it was and only change the down_interruptible() to down()?
Ben.
static int dvb_frontend_get_event(struct dvb_frontend *fe,
struct dvb_frontend_event *event, int flags)
struct dvb_frontend_event *event, int flags){ struct dvb_frontend_private *fepriv = fe->frontend_priv; struct dvb_fe_events *events = &fepriv->events; @@ -249,13 +261,8 @@ static int dvb_frontend_get_event(struct if (flags & O_NONBLOCK) return -EWOULDBLOCK;
up(&fepriv->sem);ret = wait_event_interruptible (events->wait_queue,events->eventw != events->eventr);if (down_interruptible (&fepriv->sem))return -ERESTARTSYS;
ret = wait_event_interruptible(events->wait_queue,dvb_frontend_test_event(fepriv, events));if (ret < 0) return ret;