Skip to content

Commit 536de74

Browse files
jhovoldgregkh
authored andcommitted
comedi: dt9812: fix DMA buffers on stack
USB transfer buffers are typically mapped for DMA and must not be allocated on the stack or transfers will fail. Allocate proper transfer buffers in the various command helpers and return an error on short transfers instead of acting on random stack data. Note that this also fixes a stack info leak on systems where DMA is not used as 32 bytes are always sent to the device regardless of how short the command is. Fixes: 63274cd ("Staging: comedi: add usb dt9812 driver") Cc: stable@vger.kernel.org # 2.6.29 Reviewed-by: Ian Abbott <abbotti@mev.co.uk> Signed-off-by: Johan Hovold <johan@kernel.org> Link: https://lore.kernel.org/r/20211027093529.30896-3-johan@kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 907767d commit 536de74

1 file changed

Lines changed: 86 additions & 29 deletions

File tree

drivers/comedi/drivers/dt9812.c

Lines changed: 86 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include <linux/kernel.h>
3333
#include <linux/module.h>
3434
#include <linux/errno.h>
35+
#include <linux/slab.h>
3536
#include <linux/uaccess.h>
3637

3738
#include "../comedi_usb.h"
@@ -237,22 +238,42 @@ static int dt9812_read_info(struct comedi_device *dev,
237238
{
238239
struct usb_device *usb = comedi_to_usb_dev(dev);
239240
struct dt9812_private *devpriv = dev->private;
240-
struct dt9812_usb_cmd cmd;
241+
struct dt9812_usb_cmd *cmd;
242+
size_t tbuf_size;
241243
int count, ret;
244+
void *tbuf;
242245

243-
cmd.cmd = cpu_to_le32(DT9812_R_FLASH_DATA);
244-
cmd.u.flash_data_info.address =
246+
tbuf_size = max(sizeof(*cmd), buf_size);
247+
248+
tbuf = kzalloc(tbuf_size, GFP_KERNEL);
249+
if (!tbuf)
250+
return -ENOMEM;
251+
252+
cmd = tbuf;
253+
254+
cmd->cmd = cpu_to_le32(DT9812_R_FLASH_DATA);
255+
cmd->u.flash_data_info.address =
245256
cpu_to_le16(DT9812_DIAGS_BOARD_INFO_ADDR + offset);
246-
cmd.u.flash_data_info.numbytes = cpu_to_le16(buf_size);
257+
cmd->u.flash_data_info.numbytes = cpu_to_le16(buf_size);
247258

248259
/* DT9812 only responds to 32 byte writes!! */
249260
ret = usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr),
250-
&cmd, 32, &count, DT9812_USB_TIMEOUT);
261+
cmd, sizeof(*cmd), &count, DT9812_USB_TIMEOUT);
251262
if (ret)
252-
return ret;
263+
goto out;
264+
265+
ret = usb_bulk_msg(usb, usb_rcvbulkpipe(usb, devpriv->cmd_rd.addr),
266+
tbuf, buf_size, &count, DT9812_USB_TIMEOUT);
267+
if (!ret) {
268+
if (count == buf_size)
269+
memcpy(buf, tbuf, buf_size);
270+
else
271+
ret = -EREMOTEIO;
272+
}
273+
out:
274+
kfree(tbuf);
253275

254-
return usb_bulk_msg(usb, usb_rcvbulkpipe(usb, devpriv->cmd_rd.addr),
255-
buf, buf_size, &count, DT9812_USB_TIMEOUT);
276+
return ret;
256277
}
257278

258279
static int dt9812_read_multiple_registers(struct comedi_device *dev,
@@ -261,22 +282,42 @@ static int dt9812_read_multiple_registers(struct comedi_device *dev,
261282
{
262283
struct usb_device *usb = comedi_to_usb_dev(dev);
263284
struct dt9812_private *devpriv = dev->private;
264-
struct dt9812_usb_cmd cmd;
285+
struct dt9812_usb_cmd *cmd;
265286
int i, count, ret;
287+
size_t buf_size;
288+
void *buf;
266289

267-
cmd.cmd = cpu_to_le32(DT9812_R_MULTI_BYTE_REG);
268-
cmd.u.read_multi_info.count = reg_count;
290+
buf_size = max_t(size_t, sizeof(*cmd), reg_count);
291+
292+
buf = kzalloc(buf_size, GFP_KERNEL);
293+
if (!buf)
294+
return -ENOMEM;
295+
296+
cmd = buf;
297+
298+
cmd->cmd = cpu_to_le32(DT9812_R_MULTI_BYTE_REG);
299+
cmd->u.read_multi_info.count = reg_count;
269300
for (i = 0; i < reg_count; i++)
270-
cmd.u.read_multi_info.address[i] = address[i];
301+
cmd->u.read_multi_info.address[i] = address[i];
271302

272303
/* DT9812 only responds to 32 byte writes!! */
273304
ret = usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr),
274-
&cmd, 32, &count, DT9812_USB_TIMEOUT);
305+
cmd, sizeof(*cmd), &count, DT9812_USB_TIMEOUT);
275306
if (ret)
276-
return ret;
307+
goto out;
308+
309+
ret = usb_bulk_msg(usb, usb_rcvbulkpipe(usb, devpriv->cmd_rd.addr),
310+
buf, reg_count, &count, DT9812_USB_TIMEOUT);
311+
if (!ret) {
312+
if (count == reg_count)
313+
memcpy(value, buf, reg_count);
314+
else
315+
ret = -EREMOTEIO;
316+
}
317+
out:
318+
kfree(buf);
277319

278-
return usb_bulk_msg(usb, usb_rcvbulkpipe(usb, devpriv->cmd_rd.addr),
279-
value, reg_count, &count, DT9812_USB_TIMEOUT);
320+
return ret;
280321
}
281322

282323
static int dt9812_write_multiple_registers(struct comedi_device *dev,
@@ -285,19 +326,27 @@ static int dt9812_write_multiple_registers(struct comedi_device *dev,
285326
{
286327
struct usb_device *usb = comedi_to_usb_dev(dev);
287328
struct dt9812_private *devpriv = dev->private;
288-
struct dt9812_usb_cmd cmd;
329+
struct dt9812_usb_cmd *cmd;
289330
int i, count;
331+
int ret;
332+
333+
cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
334+
if (!cmd)
335+
return -ENOMEM;
290336

291-
cmd.cmd = cpu_to_le32(DT9812_W_MULTI_BYTE_REG);
292-
cmd.u.read_multi_info.count = reg_count;
337+
cmd->cmd = cpu_to_le32(DT9812_W_MULTI_BYTE_REG);
338+
cmd->u.read_multi_info.count = reg_count;
293339
for (i = 0; i < reg_count; i++) {
294-
cmd.u.write_multi_info.write[i].address = address[i];
295-
cmd.u.write_multi_info.write[i].value = value[i];
340+
cmd->u.write_multi_info.write[i].address = address[i];
341+
cmd->u.write_multi_info.write[i].value = value[i];
296342
}
297343

298344
/* DT9812 only responds to 32 byte writes!! */
299-
return usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr),
300-
&cmd, 32, &count, DT9812_USB_TIMEOUT);
345+
ret = usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr),
346+
cmd, sizeof(*cmd), &count, DT9812_USB_TIMEOUT);
347+
kfree(cmd);
348+
349+
return ret;
301350
}
302351

303352
static int dt9812_rmw_multiple_registers(struct comedi_device *dev,
@@ -306,17 +355,25 @@ static int dt9812_rmw_multiple_registers(struct comedi_device *dev,
306355
{
307356
struct usb_device *usb = comedi_to_usb_dev(dev);
308357
struct dt9812_private *devpriv = dev->private;
309-
struct dt9812_usb_cmd cmd;
358+
struct dt9812_usb_cmd *cmd;
310359
int i, count;
360+
int ret;
361+
362+
cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
363+
if (!cmd)
364+
return -ENOMEM;
311365

312-
cmd.cmd = cpu_to_le32(DT9812_RMW_MULTI_BYTE_REG);
313-
cmd.u.rmw_multi_info.count = reg_count;
366+
cmd->cmd = cpu_to_le32(DT9812_RMW_MULTI_BYTE_REG);
367+
cmd->u.rmw_multi_info.count = reg_count;
314368
for (i = 0; i < reg_count; i++)
315-
cmd.u.rmw_multi_info.rmw[i] = rmw[i];
369+
cmd->u.rmw_multi_info.rmw[i] = rmw[i];
316370

317371
/* DT9812 only responds to 32 byte writes!! */
318-
return usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr),
319-
&cmd, 32, &count, DT9812_USB_TIMEOUT);
372+
ret = usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr),
373+
cmd, sizeof(*cmd), &count, DT9812_USB_TIMEOUT);
374+
kfree(cmd);
375+
376+
return ret;
320377
}
321378

322379
static int dt9812_digital_in(struct comedi_device *dev, u8 *bits)

0 commit comments

Comments
 (0)