[PATCH] media: vp702x: set up the state buffer before the adapter

[PATCH] media: vp702x: set up the state buffer before the adapter

[Yogesh Gaur]

vp702x_usb_probe() allocates st->buf and initializes st->buf_mutex only
after dvb_usb_device_init() has returned.
dvb_usb_device_init() calls dvb_usb_adapter_init() ->
dvb_usb_adapter_dvb_init(), which invokes the props.read_mac_address
callback. vp702x_read_mac_addr() therefore runs while the device state
is still all zeroes from the kzalloc() in dvb_usb_init(), and locks a
mutex that has never been initialized:

	DEBUG_LOCKS_WARN_ON(lock->magic != lock)
	WARNING: kernel/locking/mutex.c:625 at __mutex_lock+0x947/0x1bd0 kernel/locking/mutex.c:821
	Call Trace:
	 <TASK>
	 vp702x_read_mac_addr+0x51/0x130 drivers/media/usb/dvb-usb/vp702x.c:297
	 dvb_usb_adapter_dvb_init+0x2dd/0x860 drivers/media/usb/dvb-usb/dvb-usb-dvb.c:165
	 dvb_usb_adapter_init drivers/media/usb/dvb-usb/dvb-usb-init.c:86 [inline]
	 dvb_usb_init drivers/media/usb/dvb-usb/dvb-usb-init.c:186 [inline]
	 dvb_usb_device_init.cold+0xbd5/0x14bb drivers/media/usb/dvb-usb/dvb-usb-init.c:310
	 vp702x_usb_probe+0x8d/0x210 drivers/media/usb/dvb-usb/vp702x.c:342
	 usb_probe_interface+0x303/0x8f0 drivers/usb/core/driver.c:396
	 </TASK>

Besides taking an uninitialized mutex, vp702x_read_mac_addr() also
hands &buf[i - 6] to vp702x_usb_in_op() with st->buf still NULL.

The dvb-usb core already has a hook for this: props.priv_init is called
right after d->priv has been allocated and before any adapter is
brought up, with props.priv_destroy as its counterpart. Move the buffer
setup and teardown there, which reduces ->probe() and ->disconnect() to
the plain core calls.

priv_destroy() runs after dvb_usb_adapter_exit(), so no adapter can be
using the buffer by then and the buf_mutex that used to be held across
the kfree() in ->disconnect() is no longer needed.

Fixes: 8ea793aa7361 ("[media] vp702x: use preallocated buffer")
Reported-by: syzbot+477f9c41b0a7d90fb9cc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=477f9c41b0a7d90fb9cc
Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
---
 drivers/media/usb/dvb-usb/vp702x.c | 50 +++++++++++++-----------------
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/vp702x.c b/drivers/media/usb/dvb-usb/vp702x.c
index 034b0652b9a1..5a7ca0b77a77 100644
--- a/drivers/media/usb/dvb-usb/vp702x.c
+++ b/drivers/media/usb/dvb-usb/vp702x.c
@@ -330,43 +330,35 @@ static int vp702x_frontend_attach(struct dvb_usb_adapter *adap)
 	return 0;
 }
 
-static struct dvb_usb_device_properties vp702x_properties;
-
-static int vp702x_usb_probe(struct usb_interface *intf,
-		const struct usb_device_id *id)
+static int vp702x_priv_init(struct dvb_usb_device *d)
 {
-	struct dvb_usb_device *d;
-	struct vp702x_device_state *st;
-	int ret;
-
-	ret = dvb_usb_device_init(intf, &vp702x_properties,
-				   THIS_MODULE, &d, adapter_nr);
-	if (ret)
-		goto out;
+	struct vp702x_device_state *st = d->priv;
 
-	st = d->priv;
+	mutex_init(&st->buf_mutex);
 	st->buf_len = 16;
 	st->buf = kmalloc(st->buf_len, GFP_KERNEL);
-	if (!st->buf) {
-		ret = -ENOMEM;
-		dvb_usb_device_exit(intf);
-		goto out;
-	}
-	mutex_init(&st->buf_mutex);
-
-out:
-	return ret;
+	if (!st->buf)
+		return -ENOMEM;
 
+	return 0;
 }
 
-static void vp702x_usb_disconnect(struct usb_interface *intf)
+static void vp702x_priv_destroy(struct dvb_usb_device *d)
 {
-	struct dvb_usb_device *d = usb_get_intfdata(intf);
 	struct vp702x_device_state *st = d->priv;
-	mutex_lock(&st->buf_mutex);
+
 	kfree(st->buf);
-	mutex_unlock(&st->buf_mutex);
-	dvb_usb_device_exit(intf);
+	st->buf = NULL;
+	mutex_destroy(&st->buf_mutex);
+}
+
+static struct dvb_usb_device_properties vp702x_properties;
+
+static int vp702x_usb_probe(struct usb_interface *intf,
+			    const struct usb_device_id *id)
+{
+	return dvb_usb_device_init(intf, &vp702x_properties,
+				   THIS_MODULE, NULL, adapter_nr);
 }
 
 enum {
@@ -390,6 +382,8 @@ static struct dvb_usb_device_properties vp702x_properties = {
 	.no_reconnect        = 1,
 
 	.size_of_priv     = sizeof(struct vp702x_device_state),
+	.priv_init        = vp702x_priv_init,
+	.priv_destroy     = vp702x_priv_destroy,
 
 	.num_adapters = 1,
 	.adapter = {
@@ -443,7 +437,7 @@ static struct dvb_usb_device_properties vp702x_properties = {
 static struct usb_driver vp702x_usb_driver = {
 	.name		= "dvb_usb_vp702x",
 	.probe		= vp702x_usb_probe,
-	.disconnect	= vp702x_usb_disconnect,
+	.disconnect	= dvb_usb_device_exit,
 	.id_table	= vp702x_usb_table,
 };
 
-- 
2.55.0.windows.5