Skip to content

Commit

Permalink
[media] pctv452e: move buffer to heap, no mutex
Browse files Browse the repository at this point in the history 
commit 73d5c5c ("[media] pctv452e: don't do DMA on stack") caused
a NULL pointer dereference which occurs when dvb_usb_init()
calls dvb_usb_device_power_ctrl() for the first time, before the
frontend has been attached. It also caused a recursive deadlock because
tt3650_ci_msg_locked() has already locked the mutex.

So, partially revert it, but move the buffer to the heap
(DMA capable), not to the stack (may not be DMA capable).
Instead of sharing one buffer which needs mutex protection,
do a new heap allocation for each call.

Fixes: commit 73d5c5c ("[media] pctv452e: don't do DMA on stack")

Cc: stable@vger.kernel.org # For Kernel 4.9
Signed-off-by: Max Kellermann <max.kellermann@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
  • Loading branch information
Max Kellermann authored and Mauro Carvalho Chehab committed Dec 21, 2016
1 parent 78ccbf9 commit 48775cb
Showing 1 changed file with 72 additions and 61 deletions.
133 changes: 72 additions & 61 deletions drivers/media/usb/dvb-usb/pctv452e.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,13 @@ struct pctv452e_state {
u8 c; /* transaction counter, wraps around... */
u8 initialized; /* set to 1 if 0x15 has been sent */
u16 last_rc_key;

unsigned char data[80];
};

static int tt3650_ci_msg(struct dvb_usb_device *d, u8 cmd, u8 *data,
unsigned int write_len, unsigned int read_len)
{
struct pctv452e_state *state = (struct pctv452e_state *)d->priv;
u8 *buf;
u8 id;
unsigned int rlen;
int ret;
Expand All @@ -114,36 +113,39 @@ static int tt3650_ci_msg(struct dvb_usb_device *d, u8 cmd, u8 *data,
return -EIO;
}

mutex_lock(&state->ca_mutex);
buf = kmalloc(64, GFP_KERNEL);
if (!buf)
return -ENOMEM;

id = state->c++;

state->data[0] = SYNC_BYTE_OUT;
state->data[1] = id;
state->data[2] = cmd;
state->data[3] = write_len;
buf[0] = SYNC_BYTE_OUT;
buf[1] = id;
buf[2] = cmd;
buf[3] = write_len;

memcpy(state->data + 4, data, write_len);
memcpy(buf + 4, data, write_len);

rlen = (read_len > 0) ? 64 : 0;
ret = dvb_usb_generic_rw(d, state->data, 4 + write_len,
state->data, rlen, /* delay_ms */ 0);
ret = dvb_usb_generic_rw(d, buf, 4 + write_len,
buf, rlen, /* delay_ms */ 0);
if (0 != ret)
goto failed;

ret = -EIO;
if (SYNC_BYTE_IN != state->data[0] || id != state->data[1])
if (SYNC_BYTE_IN != buf[0] || id != buf[1])
goto failed;

memcpy(data, state->data + 4, read_len);
memcpy(data, buf + 4, read_len);

mutex_unlock(&state->ca_mutex);
kfree(buf);
return 0;

failed:
err("CI error %d; %02X %02X %02X -> %*ph.",
ret, SYNC_BYTE_OUT, id, cmd, 3, state->data);
ret, SYNC_BYTE_OUT, id, cmd, 3, buf);

mutex_unlock(&state->ca_mutex);
kfree(buf);
return ret;
}

Expand Down Expand Up @@ -410,53 +412,57 @@ static int pctv452e_i2c_msg(struct dvb_usb_device *d, u8 addr,
u8 *rcv_buf, u8 rcv_len)
{
struct pctv452e_state *state = (struct pctv452e_state *)d->priv;
u8 *buf;
u8 id;
int ret;

mutex_lock(&state->ca_mutex);
buf = kmalloc(64, GFP_KERNEL);
if (!buf)
return -ENOMEM;

id = state->c++;

ret = -EINVAL;
if (snd_len > 64 - 7 || rcv_len > 64 - 7)
goto failed;

state->data[0] = SYNC_BYTE_OUT;
state->data[1] = id;
state->data[2] = PCTV_CMD_I2C;
state->data[3] = snd_len + 3;
state->data[4] = addr << 1;
state->data[5] = snd_len;
state->data[6] = rcv_len;
buf[0] = SYNC_BYTE_OUT;
buf[1] = id;
buf[2] = PCTV_CMD_I2C;
buf[3] = snd_len + 3;
buf[4] = addr << 1;
buf[5] = snd_len;
buf[6] = rcv_len;

memcpy(state->data + 7, snd_buf, snd_len);
memcpy(buf + 7, snd_buf, snd_len);

ret = dvb_usb_generic_rw(d, state->data, 7 + snd_len,
state->data, /* rcv_len */ 64,
ret = dvb_usb_generic_rw(d, buf, 7 + snd_len,
buf, /* rcv_len */ 64,
/* delay_ms */ 0);
if (ret < 0)
goto failed;

/* TT USB protocol error. */
ret = -EIO;
if (SYNC_BYTE_IN != state->data[0] || id != state->data[1])
if (SYNC_BYTE_IN != buf[0] || id != buf[1])
goto failed;

/* I2C device didn't respond as expected. */
ret = -EREMOTEIO;
if (state->data[5] < snd_len || state->data[6] < rcv_len)
if (buf[5] < snd_len || buf[6] < rcv_len)
goto failed;

memcpy(rcv_buf, state->data + 7, rcv_len);
mutex_unlock(&state->ca_mutex);
memcpy(rcv_buf, buf + 7, rcv_len);

kfree(buf);
return rcv_len;

failed:
err("I2C error %d; %02X %02X %02X %02X %02X -> %*ph",
ret, SYNC_BYTE_OUT, id, addr << 1, snd_len, rcv_len,
7, state->data);
7, buf);

mutex_unlock(&state->ca_mutex);
kfree(buf);
return ret;
}

Expand Down Expand Up @@ -505,7 +511,7 @@ static u32 pctv452e_i2c_func(struct i2c_adapter *adapter)
static int pctv452e_power_ctrl(struct dvb_usb_device *d, int i)
{
struct pctv452e_state *state = (struct pctv452e_state *)d->priv;
u8 *rx;
u8 *b0, *rx;
int ret;

info("%s: %d\n", __func__, i);
Expand All @@ -516,86 +522,91 @@ static int pctv452e_power_ctrl(struct dvb_usb_device *d, int i)
if (state->initialized)
return 0;

rx = kmalloc(PCTV_ANSWER_LEN, GFP_KERNEL);
if (!rx)
b0 = kmalloc(5 + PCTV_ANSWER_LEN, GFP_KERNEL);
if (!b0)
return -ENOMEM;

mutex_lock(&state->ca_mutex);
rx = b0 + 5;

/* hmm where shoud this should go? */
ret = usb_set_interface(d->udev, 0, ISOC_INTERFACE_ALTERNATIVE);
if (ret != 0)
info("%s: Warning set interface returned: %d\n",
__func__, ret);

/* this is a one-time initialization, dont know where to put */
state->data[0] = 0xaa;
state->data[1] = state->c++;
state->data[2] = PCTV_CMD_RESET;
state->data[3] = 1;
state->data[4] = 0;
b0[0] = 0xaa;
b0[1] = state->c++;
b0[2] = PCTV_CMD_RESET;
b0[3] = 1;
b0[4] = 0;
/* reset board */
ret = dvb_usb_generic_rw(d, state->data, 5, rx, PCTV_ANSWER_LEN, 0);
ret = dvb_usb_generic_rw(d, b0, 5, rx, PCTV_ANSWER_LEN, 0);
if (ret)
goto ret;

state->data[1] = state->c++;
state->data[4] = 1;
b0[1] = state->c++;
b0[4] = 1;
/* reset board (again?) */
ret = dvb_usb_generic_rw(d, state->data, 5, rx, PCTV_ANSWER_LEN, 0);
ret = dvb_usb_generic_rw(d, b0, 5, rx, PCTV_ANSWER_LEN, 0);
if (ret)
goto ret;

state->initialized = 1;

ret:
mutex_unlock(&state->ca_mutex);
kfree(rx);
kfree(b0);
return ret;
}

static int pctv452e_rc_query(struct dvb_usb_device *d)
{
struct pctv452e_state *state = (struct pctv452e_state *)d->priv;
u8 *b, *rx;
int ret, i;
u8 id;

mutex_lock(&state->ca_mutex);
b = kmalloc(CMD_BUFFER_SIZE + PCTV_ANSWER_LEN, GFP_KERNEL);
if (!b)
return -ENOMEM;

rx = b + CMD_BUFFER_SIZE;

id = state->c++;

/* prepare command header */
state->data[0] = SYNC_BYTE_OUT;
state->data[1] = id;
state->data[2] = PCTV_CMD_IR;
state->data[3] = 0;
b[0] = SYNC_BYTE_OUT;
b[1] = id;
b[2] = PCTV_CMD_IR;
b[3] = 0;

/* send ir request */
ret = dvb_usb_generic_rw(d, state->data, 4,
state->data, PCTV_ANSWER_LEN, 0);
ret = dvb_usb_generic_rw(d, b, 4, rx, PCTV_ANSWER_LEN, 0);
if (ret != 0)
goto ret;

if (debug > 3) {
info("%s: read: %2d: %*ph: ", __func__, ret, 3, state->data);
for (i = 0; (i < state->data[3]) && ((i + 3) < PCTV_ANSWER_LEN); i++)
info(" %02x", state->data[i + 3]);
info("%s: read: %2d: %*ph: ", __func__, ret, 3, rx);
for (i = 0; (i < rx[3]) && ((i+3) < PCTV_ANSWER_LEN); i++)
info(" %02x", rx[i+3]);

info("\n");
}

if ((state->data[3] == 9) && (state->data[12] & 0x01)) {
if ((rx[3] == 9) && (rx[12] & 0x01)) {
/* got a "press" event */
state->last_rc_key = RC_SCANCODE_RC5(state->data[7], state->data[6]);
state->last_rc_key = RC_SCANCODE_RC5(rx[7], rx[6]);
if (debug > 2)
info("%s: cmd=0x%02x sys=0x%02x\n",
__func__, state->data[6], state->data[7]);
__func__, rx[6], rx[7]);

rc_keydown(d->rc_dev, RC_TYPE_RC5, state->last_rc_key, 0);
} else if (state->last_rc_key) {
rc_keyup(d->rc_dev);
state->last_rc_key = 0;
}
ret:
mutex_unlock(&state->ca_mutex);
kfree(b);
return ret;
}

Expand Down

0 comments on commit 48775cb

Please sign in to comment.