Hi,
These patches should address all remaining instances of DMA buffers on stack
(being passed to usb_control_msg) for the usb serial drivers. They also fix a
few endianess bugs I found along the way and do some minor clean up.
Note that I have not been able to test them on actual hardware, so please have
a go if you happen to have access to one of these devices.
Also note that these patches do not include the DMA-buffer-on-stack fix for
ftdi_sio i submitted a few days ago.
Cheers,
Johan
Johan Hovold (14):
USB: ch341: replace printk warnings with dev_err
USB: ch341: fix DMA buffer on stack
USB: ch341: use le16_to_cpup to be explicit about endianess
USB: cypress_m8: fix DMA buffer on stack
USB: cypress_m8: fix endianess bug
USB: io_ti: fix DMA buffers on stack
USB: keyspan_pda: fix DMA buffers on stack
USB: kl5kusb105: fix DMA buffers on stack
USB: mct_u232: fix DMA buffers on stack
USB: mos7720: fix DMA buffers on stack and clean up send_mos_cmd
USB: mos7840: fix DMA buffers on stack and endianess bugs
USB: oti6858: fix DMA buffer on stack
USB: visor: fix DMA buffers on stack
USB: kobil_sct: clean up kobil_set_termios
drivers/usb/serial/ch341.c | 24 +++++++++----
drivers/usb/serial/cypress_m8.c | 35 +++++++++++---------
drivers/usb/serial/io_ti.c | 66 +++++++++++++++++++++++++++----------
drivers/usb/serial/keyspan_pda.c | 45 ++++++++++++++++++-------
drivers/usb/serial/kl5kusb105.c | 63 +++++++++++++++++++++++++----------
drivers/usb/serial/kobil_sct.c | 22 ++----------
drivers/usb/serial/mct_u232.c | 67 ++++++++++++++++++++++++++++++-------
drivers/usb/serial/mos7720.c | 34 +++++++++---------
drivers/usb/serial/mos7840.c | 22 ++++++++++--
drivers/usb/serial/oti6858.c | 13 ++++++--
drivers/usb/serial/visor.c | 26 ++++++++++----
11 files changed, 282 insertions(+), 135 deletions(-)
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/ch341.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 59eff72..5e455f4 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -399,8 +399,8 @@ static void ch341_break_ctl(struct tty_struct *tty, int break_state)
r = ch341_control_in(port->serial->dev, CH341_REQ_READ_REG,
ch341_break_reg, 0, break_reg, sizeof(break_reg));
if (r < 0) {
- printk(KERN_WARNING "%s: USB control read error whilst getting"
- " break register contents.\n", __FILE__);
+ dev_err(&port->dev, "%s - USB control read error (%d)\n",
+ __func__, r);
return;
}
dbg("%s - initial ch341 break register contents - reg1: %x, reg2: %x",
@@ -420,8 +420,8 @@ static void ch341_break_ctl(struct tty_struct *tty, int break_state)
r = ch341_control_out(port->serial->dev, CH341_REQ_WRITE_REG,
ch341_break_reg, reg_contents);
if (r < 0)
- printk(KERN_WARNING "%s: USB control write error whilst setting"
- " break register contents.\n", __FILE__);
+ dev_err(&port->dev, "%s - USB control write error (%d)\n",
+ __func__, r);
}
static int ch341_tiocmset(struct tty_struct *tty, struct file *file,
--
1.6.6
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/ch341.c | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 5e455f4..2942c6d 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -392,16 +392,22 @@ static void ch341_break_ctl(struct tty_struct *tty, int break_state)
struct usb_serial_port *port = tty->driver_data;
int r;
uint16_t reg_contents;
- uint8_t break_reg[2];
+ uint8_t *break_reg;
dbg("%s()", __func__);
+ break_reg = kmalloc(2, GFP_KERNEL);
+ if (!break_reg) {
+ dev_err(&port->dev, "%s - kmalloc failed\n", __func__);
+ return;
+ }
+
r = ch341_control_in(port->serial->dev, CH341_REQ_READ_REG,
- ch341_break_reg, 0, break_reg, sizeof(break_reg));
+ ch341_break_reg, 0, break_reg, 2);
if (r < 0) {
dev_err(&port->dev, "%s - USB control read error (%d)\n",
__func__, r);
- return;
+ goto out;
}
dbg("%s - initial ch341 break register contents - reg1: %x, reg2: %x",
__func__, break_reg[0], break_reg[1]);
@@ -422,6 +428,8 @@ static void ch341_break_ctl(struct tty_struct *tty, int break_state)
if (r < 0)
dev_err(&port->dev, "%s - USB control write error (%d)\n",
__func__, r);
+out:
+ kfree(break_reg);
}
static int ch341_tiocmset(struct tty_struct *tty, struct file *file,
--
1.6.6
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/ch341.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 2942c6d..4fd01d6 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -422,7 +422,7 @@ static void ch341_break_ctl(struct tty_struct *tty, int break_state)
}
dbg("%s - New ch341 break register contents - reg1: %x, reg2: %x",
__func__, break_reg[0], break_reg[1]);
- reg_contents = (uint16_t)break_reg[0] | ((uint16_t)break_reg[1] << 8);
+ reg_contents = le16_to_cpup((uint16_t *)break_reg);
r = ch341_control_out(port->serial->dev, CH341_REQ_WRITE_REG,
ch341_break_reg, reg_contents);
if (r < 0)
--
1.6.6
Cc: Lonnie Mendez <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/cypress_m8.c | 31 +++++++++++++++++--------------
1 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c
index a591ebe..478983f 100644
--- a/drivers/usb/serial/cypress_m8.c
+++ b/drivers/usb/serial/cypress_m8.c
@@ -344,7 +344,8 @@ static int cypress_serial_control(struct tty_struct *tty,
{
int new_baudrate = 0, retval = 0, tries = 0;
struct cypress_private *priv;
- __u8 feature_buffer[5];
+ u8 *feature_buffer;
+ const unsigned int feature_len = 5;
unsigned long flags;
dbg("%s", __func__);
@@ -354,6 +355,10 @@ static int cypress_serial_control(struct tty_struct *tty,
if (!priv->comm_is_ok)
return -ENODEV;
+ feature_buffer = kcalloc(feature_len, sizeof(u8), GFP_KERNEL);
+ if (!feature_buffer)
+ return -ENOMEM;
+
switch (cypress_request_type) {
case CYPRESS_SET_CONFIG:
new_baudrate = priv->baud_rate;
@@ -373,7 +378,6 @@ static int cypress_serial_control(struct tty_struct *tty,
dbg("%s - baud rate is being sent as %d",
__func__, new_baudrate);
- memset(feature_buffer, 0, sizeof(feature_buffer));
/* fill the feature_buffer with new configuration */
*((u_int32_t *)feature_buffer) = new_baudrate;
feature_buffer[4] |= data_bits; /* assign data bits in 2 bit space ( max 3 ) */
@@ -397,15 +401,15 @@ static int cypress_serial_control(struct tty_struct *tty,
HID_REQ_SET_REPORT,
USB_DIR_OUT | USB_RECIP_INTERFACE | USB_TYPE_CLASS,
0x0300, 0, feature_buffer,
- sizeof(feature_buffer), 500);
+ feature_len, 500);
if (tries++ >= 3)
break;
- } while (retval != sizeof(feature_buffer) &&
+ } while (retval != feature_len &&
retval != -ENODEV);
- if (retval != sizeof(feature_buffer)) {
+ if (retval != feature_len) {
dev_err(&port->dev, "%s - failed sending serial "
"line settings - %d\n", __func__, retval);
cypress_set_dead(port);
@@ -425,30 +429,28 @@ static int cypress_serial_control(struct tty_struct *tty,
/* Not implemented for this device,
and if we try to do it we're likely
to crash the hardware. */
- return -ENOTTY;
+ retval = -ENOTTY;
+ goto out;
}
dbg("%s - retreiving serial line settings", __func__);
- /* set initial values in feature buffer */
- memset(feature_buffer, 0, sizeof(feature_buffer));
-
do {
retval = usb_control_msg(port->serial->dev,
usb_rcvctrlpipe(port->serial->dev, 0),
HID_REQ_GET_REPORT,
USB_DIR_IN | USB_RECIP_INTERFACE | USB_TYPE_CLASS,
0x0300, 0, feature_buffer,
- sizeof(feature_buffer), 500);
+ feature_len, 500);
if (tries++ >= 3)
break;
- } while (retval != sizeof(feature_buffer)
+ } while (retval != feature_len
&& retval != -ENODEV);
- if (retval != sizeof(feature_buffer)) {
+ if (retval != feature_len) {
dev_err(&port->dev, "%s - failed to retrieve serial "
"line settings - %d\n", __func__, retval);
cypress_set_dead(port);
- return retval;
+ goto out;
} else {
spin_lock_irqsave(&priv->lock, flags);
/* store the config in one byte, and later
@@ -461,7 +463,8 @@ static int cypress_serial_control(struct tty_struct *tty,
spin_lock_irqsave(&priv->lock, flags);
++priv->cmd_count;
spin_unlock_irqrestore(&priv->lock, flags);
-
+out:
+ kfree(feature_buffer);
return retval;
} /* cypress_serial_control */
--
1.6.6
Cc: Lonnie Mendez <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/cypress_m8.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c
index 478983f..b34c812 100644
--- a/drivers/usb/serial/cypress_m8.c
+++ b/drivers/usb/serial/cypress_m8.c
@@ -379,7 +379,7 @@ static int cypress_serial_control(struct tty_struct *tty,
__func__, new_baudrate);
/* fill the feature_buffer with new configuration */
- *((u_int32_t *)feature_buffer) = new_baudrate;
+ *((u32 *)feature_buffer) = cpu_to_le32(new_baudrate);
feature_buffer[4] |= data_bits; /* assign data bits in 2 bit space ( max 3 ) */
/* 1 bit gap */
feature_buffer[4] |= (stop_bits << 3); /* assign stop bits in 1 bit space */
@@ -456,7 +456,7 @@ static int cypress_serial_control(struct tty_struct *tty,
/* store the config in one byte, and later
use bit masks to check values */
priv->current_config = feature_buffer[4];
- priv->baud_rate = *((u_int32_t *)feature_buffer);
+ priv->baud_rate = le32_to_cpup((u32 *)feature_buffer);
spin_unlock_irqrestore(&priv->lock, flags);
}
}
--
1.6.6
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/io_ti.c | 66 ++++++++++++++++++++++++++++++++------------
1 files changed, 48 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index d4cc0f7..1691f07 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -413,11 +413,18 @@ static int write_boot_mem(struct edgeport_serial *serial,
{
int status = 0;
int i;
- __u8 temp;
+ u8 *temp;
/* Must do a read before write */
if (!serial->TiReadI2C) {
- status = read_boot_mem(serial, 0, 1, &temp);
+ temp = kmalloc(1, GFP_KERNEL);
+ if (!temp) {
+ dev_err(&serial->serial->dev->dev,
+ "%s - out of memory\n", __func__);
+ return -ENOMEM;
+ }
+ status = read_boot_mem(serial, 0, 1, temp);
+ kfree(temp);
if (status)
return status;
}
@@ -935,37 +942,47 @@ static int build_i2c_fw_hdr(__u8 *header, struct device *dev)
static int i2c_type_bootmode(struct edgeport_serial *serial)
{
int status;
- __u8 data;
+ u8 *data;
+
+ data = kmalloc(1, GFP_KERNEL);
+ if (!data) {
+ dev_err(&serial->serial->dev->dev,
+ "%s - out of memory\n", __func__);
+ return -ENOMEM;
+ }
/* Try to read type 2 */
status = ti_vread_sync(serial->serial->dev, UMPC_MEMORY_READ,
- DTK_ADDR_SPACE_I2C_TYPE_II, 0, &data, 0x01);
+ DTK_ADDR_SPACE_I2C_TYPE_II, 0, data, 0x01);
if (status)
dbg("%s - read 2 status error = %d", __func__, status);
else
- dbg("%s - read 2 data = 0x%x", __func__, data);
- if ((!status) && (data == UMP5152 || data == UMP3410)) {
+ dbg("%s - read 2 data = 0x%x", __func__, *data);
+ if ((!status) && (*data == UMP5152 || *data == UMP3410)) {
dbg("%s - ROM_TYPE_II", __func__);
serial->TI_I2C_Type = DTK_ADDR_SPACE_I2C_TYPE_II;
- return 0;
+ goto out;
}
/* Try to read type 3 */
status = ti_vread_sync(serial->serial->dev, UMPC_MEMORY_READ,
- DTK_ADDR_SPACE_I2C_TYPE_III, 0, &data, 0x01);
+ DTK_ADDR_SPACE_I2C_TYPE_III, 0, data, 0x01);
if (status)
dbg("%s - read 3 status error = %d", __func__, status);
else
- dbg("%s - read 2 data = 0x%x", __func__, data);
- if ((!status) && (data == UMP5152 || data == UMP3410)) {
+ dbg("%s - read 2 data = 0x%x", __func__, *data);
+ if ((!status) && (*data == UMP5152 || *data == UMP3410)) {
dbg("%s - ROM_TYPE_III", __func__);
serial->TI_I2C_Type = DTK_ADDR_SPACE_I2C_TYPE_III;
- return 0;
+ goto out;
}
dbg("%s - Unknown", __func__);
serial->TI_I2C_Type = DTK_ADDR_SPACE_I2C_TYPE_II;
- return -ENODEV;
+ status = -ENODEV;
+out:
+ kfree(data);
+ return status;
}
static int bulk_xfer(struct usb_serial *serial, void *buffer,
@@ -1113,7 +1130,7 @@ static int download_fw(struct edgeport_serial *serial)
I2C_DESC_TYPE_FIRMWARE_BASIC, rom_desc);
if (start_address != 0) {
struct ti_i2c_firmware_rec *firmware_version;
- __u8 record;
+ u8 *record;
dbg("%s - Found Type FIRMWARE (Type 2) record",
__func__);
@@ -1165,6 +1182,15 @@ static int download_fw(struct edgeport_serial *serial)
OperationalMajorVersion,
OperationalMinorVersion);
+ record = kmalloc(1, GFP_KERNEL);
+ if (!record) {
+ dev_err(dev, "%s - out of memory.\n",
+ __func__);
+ kfree(firmware_version);
+ kfree(rom_desc);
+ kfree(ti_manuf_desc);
+ return -ENOMEM;
+ }
/* In order to update the I2C firmware we must
* change the type 2 record to type 0xF2. This
* will force the UMP to come up in Boot Mode.
@@ -1177,13 +1203,14 @@ static int download_fw(struct edgeport_serial *serial)
* firmware will update the record type from
* 0xf2 to 0x02.
*/
- record = I2C_DESC_TYPE_FIRMWARE_BLANK;
+ *record = I2C_DESC_TYPE_FIRMWARE_BLANK;
/* Change the I2C Firmware record type to
0xf2 to trigger an update */
status = write_rom(serial, start_address,
- sizeof(record), &record);
+ sizeof(*record), record);
if (status) {
+ kfree(record);
kfree(firmware_version);
kfree(rom_desc);
kfree(ti_manuf_desc);
@@ -1196,19 +1223,21 @@ static int download_fw(struct edgeport_serial *serial)
*/
status = read_rom(serial,
start_address,
- sizeof(record),
- &record);
+ sizeof(*record),
+ record);
if (status) {
+ kfree(record);
kfree(firmware_version);
kfree(rom_desc);
kfree(ti_manuf_desc);
return status;
}
- if (record != I2C_DESC_TYPE_FIRMWARE_BLANK) {
+ if (*record != I2C_DESC_TYPE_FIRMWARE_BLANK) {
dev_err(dev,
"%s - error resetting device\n",
__func__);
+ kfree(record);
kfree(firmware_version);
kfree(rom_desc);
kfree(ti_manuf_desc);
@@ -1226,6 +1255,7 @@ static int download_fw(struct edgeport_serial *serial)
__func__, status);
/* return an error on purpose. */
+ kfree(record);
kfree(firmware_version);
kfree(rom_desc);
kfree(ti_manuf_desc);
--
1.6.6
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/keyspan_pda.c | 45 +++++++++++++++++++++++++++----------
1 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c
index 1296a09..427d377 100644
--- a/drivers/usb/serial/keyspan_pda.c
+++ b/drivers/usb/serial/keyspan_pda.c
@@ -429,13 +429,20 @@ static int keyspan_pda_get_modem_info(struct usb_serial *serial,
unsigned char *value)
{
int rc;
- unsigned char data;
+ u8 *data;
+
+ data = kmalloc(1, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
rc = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
3, /* get pins */
USB_TYPE_VENDOR|USB_RECIP_INTERFACE|USB_DIR_IN,
- 0, 0, &data, 1, 2000);
+ 0, 0, data, 1, 2000);
if (rc >= 0)
- *value = data;
+ *value = *data;
+
+ kfree(data);
return rc;
}
@@ -543,7 +550,14 @@ static int keyspan_pda_write(struct tty_struct *tty,
device how much room it really has. This is done only on
scheduler time, since usb_control_msg() sleeps. */
if (count > priv->tx_room && !in_interrupt()) {
- unsigned char room;
+ u8 *room;
+
+ room = kmalloc(1, GFP_KERNEL);
+ if (!room) {
+ rc = -ENOMEM;
+ goto exit;
+ }
+
rc = usb_control_msg(serial->dev,
usb_rcvctrlpipe(serial->dev, 0),
6, /* write_room */
@@ -551,9 +565,14 @@ static int keyspan_pda_write(struct tty_struct *tty,
| USB_DIR_IN,
0, /* value: 0 means "remaining room" */
0, /* index */
- &room,
+ room,
1,
2000);
+ if (rc > 0) {
+ dbg(" roomquery says %d", *room);
+ priv->tx_room = *room;
+ }
+ kfree(room);
if (rc < 0) {
dbg(" roomquery failed");
goto exit;
@@ -563,8 +582,6 @@ static int keyspan_pda_write(struct tty_struct *tty,
rc = -EIO; /* device didn't return any data */
goto exit;
}
- dbg(" roomquery says %d", room);
- priv->tx_room = room;
}
if (count > priv->tx_room) {
/* we're about to completely fill the Tx buffer, so
@@ -684,18 +701,22 @@ static int keyspan_pda_open(struct tty_struct *tty,
struct usb_serial_port *port)
{
struct usb_serial *serial = port->serial;
- unsigned char room;
+ u8 *room;
int rc = 0;
struct keyspan_pda_private *priv;
/* find out how much room is in the Tx ring */
+ room = kmalloc(1, GFP_KERNEL);
+ if (!room)
+ return -ENOMEM;
+
rc = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
6, /* write_room */
USB_TYPE_VENDOR | USB_RECIP_INTERFACE
| USB_DIR_IN,
0, /* value */
0, /* index */
- &room,
+ room,
1,
2000);
if (rc < 0) {
@@ -708,8 +729,8 @@ static int keyspan_pda_open(struct tty_struct *tty,
goto error;
}
priv = usb_get_serial_port_data(port);
- priv->tx_room = room;
- priv->tx_throttled = room ? 0 : 1;
+ priv->tx_room = *room;
+ priv->tx_throttled = *room ? 0 : 1;
/*Start reading from the device*/
port->interrupt_in_urb->dev = serial->dev;
@@ -718,8 +739,8 @@ static int keyspan_pda_open(struct tty_struct *tty,
dbg("%s - usb_submit_urb(read int) failed", __func__);
goto error;
}
-
error:
+ kfree(room);
return rc;
}
static void keyspan_pda_close(struct usb_serial_port *port)
--
1.6.6
Cc: Oliver Neukum <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/kl5kusb105.c | 63 +++++++++++++++++++++++++++-----------
1 files changed, 45 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/serial/kl5kusb105.c b/drivers/usb/serial/kl5kusb105.c
index 3a78738..731964b 100644
--- a/drivers/usb/serial/kl5kusb105.c
+++ b/drivers/usb/serial/kl5kusb105.c
@@ -212,10 +212,19 @@ static int klsi_105_get_line_state(struct usb_serial_port *port,
unsigned long *line_state_p)
{
int rc;
- __u8 status_buf[KLSI_STATUSBUF_LEN] = { -1, -1};
+ u8 *status_buf;
__u16 status;
dev_info(&port->serial->dev->dev, "sending SIO Poll request\n");
+
+ status_buf = kmalloc(KLSI_STATUSBUF_LEN, GFP_KERNEL);
+ if (!status_buf) {
+ dev_err(&port->dev, "%s - out of memory for status buffer.\n",
+ __func__);
+ return -ENOMEM;
+ }
+ status_buf[0] = 0xff;
+ status_buf[1] = 0xff;
rc = usb_control_msg(port->serial->dev,
usb_rcvctrlpipe(port->serial->dev, 0),
KL5KUSB105A_SIO_POLL,
@@ -236,6 +245,8 @@ static int klsi_105_get_line_state(struct usb_serial_port *port,
*line_state_p = klsi_105_status2linestate(status);
}
+
+ kfree(status_buf);
return rc;
}
@@ -364,7 +375,7 @@ static int klsi_105_open(struct tty_struct *tty, struct usb_serial_port *port)
int rc;
int i;
unsigned long line_state;
- struct klsi_105_port_settings cfg;
+ struct klsi_105_port_settings *cfg;
unsigned long flags;
dbg("%s port %d", __func__, port->number);
@@ -376,12 +387,18 @@ static int klsi_105_open(struct tty_struct *tty, struct usb_serial_port *port)
* Then read the modem line control and store values in
* priv->line_state.
*/
- cfg.pktlen = 5;
- cfg.baudrate = kl5kusb105a_sio_b9600;
- cfg.databits = kl5kusb105a_dtb_8;
- cfg.unknown1 = 0;
- cfg.unknown2 = 1;
- klsi_105_chg_port_settings(port, &cfg);
+ cfg = kmalloc(sizeof(*cfg), GFP_KERNEL);
+ if (!cfg) {
+ dev_err(&port->dev, "%s - out of memory for config buffer.\n",
+ __func__);
+ return -ENOMEM;
+ }
+ cfg->pktlen = 5;
+ cfg->baudrate = kl5kusb105a_sio_b9600;
+ cfg->databits = kl5kusb105a_dtb_8;
+ cfg->unknown1 = 0;
+ cfg->unknown2 = 1;
+ klsi_105_chg_port_settings(port, cfg);
/* set up termios structure */
spin_lock_irqsave(&priv->lock, flags);
@@ -391,11 +408,11 @@ static int klsi_105_open(struct tty_struct *tty, struct usb_serial_port *port)
priv->termios.c_lflag = tty->termios->c_lflag;
for (i = 0; i < NCCS; i++)
priv->termios.c_cc[i] = tty->termios->c_cc[i];
- priv->cfg.pktlen = cfg.pktlen;
- priv->cfg.baudrate = cfg.baudrate;
- priv->cfg.databits = cfg.databits;
- priv->cfg.unknown1 = cfg.unknown1;
- priv->cfg.unknown2 = cfg.unknown2;
+ priv->cfg.pktlen = cfg->pktlen;
+ priv->cfg.baudrate = cfg->baudrate;
+ priv->cfg.databits = cfg->databits;
+ priv->cfg.unknown1 = cfg->unknown1;
+ priv->cfg.unknown2 = cfg->unknown2;
spin_unlock_irqrestore(&priv->lock, flags);
/* READ_ON and urb submission */
@@ -441,6 +458,7 @@ static int klsi_105_open(struct tty_struct *tty, struct usb_serial_port *port)
retval = rc;
exit:
+ kfree(cfg);
return retval;
} /* klsi_105_open */
@@ -714,10 +732,17 @@ static void klsi_105_set_termios(struct tty_struct *tty,
unsigned int old_iflag = old_termios->c_iflag;
unsigned int cflag = tty->termios->c_cflag;
unsigned int old_cflag = old_termios->c_cflag;
- struct klsi_105_port_settings cfg;
+ struct klsi_105_port_settings *cfg;
unsigned long flags;
speed_t baud;
+ cfg = kmalloc(sizeof(*cfg), GFP_KERNEL);
+ if (!cfg) {
+ dev_err(&port->dev, "%s - out of memory for config buffer.\n",
+ __func__);
+ return;
+ }
+
/* lock while we are modifying the settings */
spin_lock_irqsave(&priv->lock, flags);
@@ -793,11 +818,11 @@ static void klsi_105_set_termios(struct tty_struct *tty,
case CS5:
dbg("%s - 5 bits/byte not supported", __func__);
spin_unlock_irqrestore(&priv->lock, flags);
- return ;
+ goto err;
case CS6:
dbg("%s - 6 bits/byte not supported", __func__);
spin_unlock_irqrestore(&priv->lock, flags);
- return ;
+ goto err;
case CS7:
priv->cfg.databits = kl5kusb105a_dtb_7;
break;
@@ -856,11 +881,13 @@ static void klsi_105_set_termios(struct tty_struct *tty,
#endif
;
}
- memcpy(&cfg, &priv->cfg, sizeof(cfg));
+ memcpy(cfg, &priv->cfg, sizeof(*cfg));
spin_unlock_irqrestore(&priv->lock, flags);
/* now commit changes to device */
- klsi_105_chg_port_settings(port, &cfg);
+ klsi_105_chg_port_settings(port, cfg);
+err:
+ kfree(cfg);
} /* klsi_105_set_termios */
--
1.6.6
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/mct_u232.c | 67 +++++++++++++++++++++++++++++++++--------
1 files changed, 54 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/serial/mct_u232.c b/drivers/usb/serial/mct_u232.c
index cd009cb..67e86b0 100644
--- a/drivers/usb/serial/mct_u232.c
+++ b/drivers/usb/serial/mct_u232.c
@@ -231,19 +231,25 @@ static int mct_u232_calculate_baud_rate(struct usb_serial *serial,
static int mct_u232_set_baud_rate(struct tty_struct *tty,
struct usb_serial *serial, struct usb_serial_port *port, speed_t value)
{
+ u8 *buf;
__le32 divisor;
int rc;
- unsigned char zero_byte = 0;
- unsigned char cts_enable_byte = 0;
speed_t speed;
+ buf = kmalloc(4, GFP_KERNEL);
+ if (!buf) {
+ dev_err(&port->dev, "%s - memory allocation failed\n",
+ __func__);
+ return -ENOMEM;
+ }
+
divisor = cpu_to_le32(mct_u232_calculate_baud_rate(serial, value,
&speed));
-
+ *(u32 *)buf = divisor;
rc = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
MCT_U232_SET_BAUD_RATE_REQUEST,
MCT_U232_SET_REQUEST_TYPE,
- 0, 0, &divisor, MCT_U232_SET_BAUD_RATE_SIZE,
+ 0, 0, buf, MCT_U232_SET_BAUD_RATE_SIZE,
WDR_TIMEOUT);
if (rc < 0) /*FIXME: What value speed results */
dev_err(&port->dev, "Set BAUD RATE %d failed (error = %d)\n",
@@ -268,11 +274,11 @@ static int mct_u232_set_baud_rate(struct tty_struct *tty,
value of 1 is used by this driver), data will not be transmitted to
a device which is not asserting 'CTS'.
*/
-
+ buf[0] = 0;
rc = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
MCT_U232_SET_UNKNOWN1_REQUEST,
MCT_U232_SET_REQUEST_TYPE,
- 0, 0, &zero_byte, MCT_U232_SET_UNKNOWN1_SIZE,
+ 0, 0, buf, MCT_U232_SET_UNKNOWN1_SIZE,
WDR_TIMEOUT);
if (rc < 0)
dev_err(&port->dev, "Sending USB device request code %d "
@@ -280,76 +286,111 @@ static int mct_u232_set_baud_rate(struct tty_struct *tty,
rc);
if (port && C_CRTSCTS(tty))
- cts_enable_byte = 1;
+ buf[0] = 1;
+ else
+ buf[0] = 0;
dbg("set_baud_rate: send second control message, data = %02X",
- cts_enable_byte);
+ buf[0]);
rc = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
MCT_U232_SET_CTS_REQUEST,
MCT_U232_SET_REQUEST_TYPE,
- 0, 0, &cts_enable_byte, MCT_U232_SET_CTS_SIZE,
+ 0, 0, buf, MCT_U232_SET_CTS_SIZE,
WDR_TIMEOUT);
if (rc < 0)
dev_err(&port->dev, "Sending USB device request code %d "
"failed (error = %d)\n", MCT_U232_SET_CTS_REQUEST, rc);
+ kfree(buf);
return rc;
} /* mct_u232_set_baud_rate */
static int mct_u232_set_line_ctrl(struct usb_serial *serial, unsigned char lcr)
{
int rc;
+ u8 *buf;
+
+ buf = kmalloc(1, GFP_KERNEL);
+ if (!buf) {
+ dev_err(&serial->dev->dev,
+ "%s - memory allocation failed\n", __func__);
+ return -ENOMEM;
+ }
+ buf[0] = lcr;
rc = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
MCT_U232_SET_LINE_CTRL_REQUEST,
MCT_U232_SET_REQUEST_TYPE,
- 0, 0, &lcr, MCT_U232_SET_LINE_CTRL_SIZE,
+ 0, 0, buf, MCT_U232_SET_LINE_CTRL_SIZE,
WDR_TIMEOUT);
if (rc < 0)
dev_err(&serial->dev->dev,
"Set LINE CTRL 0x%x failed (error = %d)\n", lcr, rc);
dbg("set_line_ctrl: 0x%x", lcr);
+ kfree(buf);
return rc;
} /* mct_u232_set_line_ctrl */
static int mct_u232_set_modem_ctrl(struct usb_serial *serial,
unsigned int control_state)
{
+ u8 *buf;
int rc;
unsigned char mcr = MCT_U232_MCR_NONE;
+ buf = kmalloc(1, GFP_KERNEL);
+ if (!buf) {
+ dev_err(&serial->dev->dev,
+ "%s - memory allocation failed\n", __func__);
+ return -ENOMEM;
+ }
+
if (control_state & TIOCM_DTR)
mcr |= MCT_U232_MCR_DTR;
if (control_state & TIOCM_RTS)
mcr |= MCT_U232_MCR_RTS;
+ buf[0] = mcr;
rc = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
MCT_U232_SET_MODEM_CTRL_REQUEST,
MCT_U232_SET_REQUEST_TYPE,
- 0, 0, &mcr, MCT_U232_SET_MODEM_CTRL_SIZE,
+ 0, 0, buf, MCT_U232_SET_MODEM_CTRL_SIZE,
WDR_TIMEOUT);
if (rc < 0)
dev_err(&serial->dev->dev,
"Set MODEM CTRL 0x%x failed (error = %d)\n", mcr, rc);
dbg("set_modem_ctrl: state=0x%x ==> mcr=0x%x", control_state, mcr);
+ kfree(buf);
return rc;
} /* mct_u232_set_modem_ctrl */
static int mct_u232_get_modem_stat(struct usb_serial *serial,
unsigned char *msr)
{
+ u8 *buf;
int rc;
+
+ buf = kmalloc(1, GFP_KERNEL);
+ if (!buf) {
+ dev_err(&serial->dev->dev,
+ "%s - memory allocation failed\n", __func__);
+ *msr = 0;
+ return -ENOMEM;
+ }
+
rc = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
MCT_U232_GET_MODEM_STAT_REQUEST,
MCT_U232_GET_REQUEST_TYPE,
- 0, 0, msr, MCT_U232_GET_MODEM_STAT_SIZE,
+ 0, 0, buf, MCT_U232_GET_MODEM_STAT_SIZE,
WDR_TIMEOUT);
if (rc < 0) {
dev_err(&serial->dev->dev,
"Get MODEM STATus failed (error = %d)\n", rc);
*msr = 0;
- }
+ } else
+ *msr = buf[0];
dbg("get_modem_stat: 0x%x", *msr);
+ kfree(buf);
return rc;
} /* mct_u232_get_modem_stat */
--
1.6.6
Change data-argument type from (void *) to (u8 *) to prevent endianess
problems.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/mos7720.c | 34 +++++++++++++++++-----------------
1 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
index 763e32a..e081dc0 100644
--- a/drivers/usb/serial/mos7720.c
+++ b/drivers/usb/serial/mos7720.c
@@ -275,13 +275,11 @@ static void mos7720_bulk_out_data_callback(struct urb *urb)
* this function will be used for sending command to device
*/
static int send_mos_cmd(struct usb_serial *serial, __u8 request, __u16 value,
- __u16 index, void *data)
+ __u16 index, u8 *data)
{
int status;
- unsigned int pipe;
+ u8 *buf;
u16 product = le16_to_cpu(serial->dev->descriptor.idProduct);
- __u8 requesttype;
- __u16 size = 0x0000;
if (value < MOS_MAX_PORT) {
if (product == MOSCHIP_DEVICE_ID_7715)
@@ -298,21 +296,23 @@ static int send_mos_cmd(struct usb_serial *serial, __u8 request, __u16 value,
}
if (request == MOS_WRITE) {
- request = (__u8)MOS_WRITE;
- requesttype = (__u8)0x40;
- value = value + (__u16)*((unsigned char *)data);
- data = NULL;
- pipe = usb_sndctrlpipe(serial->dev, 0);
+ value = value + *data;
+ status = usb_control_msg(serial->dev,
+ usb_sndctrlpipe(serial->dev, 0), MOS_WRITE,
+ 0x40, value, index, NULL, 0, MOS_WDR_TIMEOUT);
} else {
- request = (__u8)MOS_READ;
- requesttype = (__u8)0xC0;
- size = 0x01;
- pipe = usb_rcvctrlpipe(serial->dev, 0);
+ buf = kmalloc(1, GFP_KERNEL);
+ if (!buf) {
+ status = -ENOMEM;
+ goto out;
+ }
+ status = usb_control_msg(serial->dev,
+ usb_rcvctrlpipe(serial->dev, 0), MOS_READ,
+ 0xc0, value, index, buf, 1, MOS_WDR_TIMEOUT);
+ *data = *buf;
+ kfree(buf);
}
-
- status = usb_control_msg(serial->dev, pipe, request, requesttype,
- value, index, data, size, MOS_WDR_TIMEOUT);
-
+out:
if (status < 0)
dbg("Command Write failed Value %x index %x\n", value, index);
--
1.6.6
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/mos7840.c | 22 ++++++++++++++++++----
1 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 2cfe245..04bef4b 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -283,12 +283,19 @@ static int mos7840_get_reg_sync(struct usb_serial_port *port, __u16 reg,
{
struct usb_device *dev = port->serial->dev;
int ret = 0;
+ u8 *buf;
+
+ buf = kmalloc(VENDOR_READ_LENGTH, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), MCS_RDREQ,
- MCS_RD_RTYPE, 0, reg, val, VENDOR_READ_LENGTH,
+ MCS_RD_RTYPE, 0, reg, buf, VENDOR_READ_LENGTH,
MOS_WDR_TIMEOUT);
+ *val = buf[0];
dbg("mos7840_get_reg_sync offset is %x, return val %x", reg, *val);
- *val = (*val) & 0x00ff;
+
+ kfree(buf);
return ret;
}
@@ -341,6 +348,11 @@ static int mos7840_get_uart_reg(struct usb_serial_port *port, __u16 reg,
struct usb_device *dev = port->serial->dev;
int ret = 0;
__u16 Wval;
+ u8 *buf;
+
+ buf = kmalloc(VENDOR_READ_LENGTH, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
/* dbg("application number is %4x",
(((__u16)port->number - (__u16)(port->serial->minor))+1)<<8); */
@@ -364,9 +376,11 @@ static int mos7840_get_uart_reg(struct usb_serial_port *port, __u16 reg,
}
}
ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), MCS_RDREQ,
- MCS_RD_RTYPE, Wval, reg, val, VENDOR_READ_LENGTH,
+ MCS_RD_RTYPE, Wval, reg, buf, VENDOR_READ_LENGTH,
MOS_WDR_TIMEOUT);
- *val = (*val) & 0x00ff;
+ *val = buf[0];
+
+ kfree(buf);
return ret;
}
--
1.6.6
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/oti6858.c | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/serial/oti6858.c b/drivers/usb/serial/oti6858.c
index c644e26..2ae97e7 100644
--- a/drivers/usb/serial/oti6858.c
+++ b/drivers/usb/serial/oti6858.c
@@ -302,7 +302,7 @@ void send_data(struct work_struct *work)
struct usb_serial_port *port = priv->port;
int count = 0, result;
unsigned long flags;
- unsigned char allow;
+ u8 *allow;
dbg("%s(port = %d)", __func__, port->number);
@@ -321,13 +321,20 @@ void send_data(struct work_struct *work)
count = port->bulk_out_size;
if (count != 0) {
+ allow = kmalloc(1, GFP_KERNEL);
+ if (!allow) {
+ dev_err(&port->dev, "%s(): kmalloc failed\n",
+ __func__);
+ return;
+ }
result = usb_control_msg(port->serial->dev,
usb_rcvctrlpipe(port->serial->dev, 0),
OTI6858_REQ_T_CHECK_TXBUFF,
OTI6858_REQ_CHECK_TXBUFF,
- count, 0, &allow, 1, 100);
- if (result != 1 || allow != 0)
+ count, 0, allow, 1, 100);
+ if (result != 1 || *allow != 0)
count = 0;
+ kfree(allow);
}
if (count == 0) {
--
1.6.6
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/visor.c | 26 ++++++++++++++++++--------
1 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/serial/visor.c b/drivers/usb/serial/visor.c
index ad1f923..178e4d9 100644
--- a/drivers/usb/serial/visor.c
+++ b/drivers/usb/serial/visor.c
@@ -807,10 +807,14 @@ static int clie_3_5_startup(struct usb_serial *serial)
{
struct device *dev = &serial->dev->dev;
int result;
- u8 data;
+ u8 *data;
dbg("%s", __func__);
+ data = kmalloc(1, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
/*
* Note that PEG-300 series devices expect the following two calls.
*/
@@ -818,36 +822,42 @@ static int clie_3_5_startup(struct usb_serial *serial)
/* get the config number */
result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
USB_REQ_GET_CONFIGURATION, USB_DIR_IN,
- 0, 0, &data, 1, 3000);
+ 0, 0, data, 1, 3000);
if (result < 0) {
dev_err(dev, "%s: get config number failed: %d\n",
__func__, result);
- return result;
+ goto out;
}
if (result != 1) {
dev_err(dev, "%s: get config number bad return length: %d\n",
__func__, result);
- return -EIO;
+ result = -EIO;
+ goto out;
}
/* get the interface number */
result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
USB_REQ_GET_INTERFACE,
USB_DIR_IN | USB_RECIP_INTERFACE,
- 0, 0, &data, 1, 3000);
+ 0, 0, data, 1, 3000);
if (result < 0) {
dev_err(dev, "%s: get interface number failed: %d\n",
__func__, result);
- return result;
+ goto out;
}
if (result != 1) {
dev_err(dev,
"%s: get interface number bad return length: %d\n",
__func__, result);
- return -EIO;
+ result = -EIO;
+ goto out;
}
- return generic_startup(serial);
+ result = generic_startup(serial);
+out:
+ kfree(data);
+
+ return result;
}
static int treo_attach(struct usb_serial *serial)
--
1.6.6
Kill string that is allocated and generated using speed and parity
settings but is never used (and never has been).
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/kobil_sct.c | 22 ++++------------------
1 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/serial/kobil_sct.c b/drivers/usb/serial/kobil_sct.c
index 45ea694..f917c5b 100644
--- a/drivers/usb/serial/kobil_sct.c
+++ b/drivers/usb/serial/kobil_sct.c
@@ -624,7 +624,6 @@ static void kobil_set_termios(struct tty_struct *tty,
unsigned short urb_val = 0;
int c_cflag = tty->termios->c_cflag;
speed_t speed;
- void *settings;
priv = usb_get_serial_port_data(port);
if (priv->device_type == KOBIL_USBTWIN_PRODUCT_ID ||
@@ -647,25 +646,13 @@ static void kobil_set_termios(struct tty_struct *tty,
}
urb_val |= (c_cflag & CSTOPB) ? SUSBCR_SPASB_2StopBits :
SUSBCR_SPASB_1StopBit;
-
- settings = kzalloc(50, GFP_KERNEL);
- if (!settings)
- return;
-
- sprintf(settings, "%d ", speed);
-
if (c_cflag & PARENB) {
- if (c_cflag & PARODD) {
+ if (c_cflag & PARODD)
urb_val |= SUSBCR_SPASB_OddParity;
- strcat(settings, "Odd Parity");
- } else {
+ else
urb_val |= SUSBCR_SPASB_EvenParity;
- strcat(settings, "Even Parity");
- }
- } else {
+ } else
urb_val |= SUSBCR_SPASB_NoParity;
- strcat(settings, "No Parity");
- }
tty->termios->c_cflag &= ~CMSPAR;
tty_encode_baud_rate(tty, speed, speed);
@@ -675,11 +662,10 @@ static void kobil_set_termios(struct tty_struct *tty,
USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT,
urb_val,
0,
- settings,
+ NULL,
0,
KOBIL_TIMEOUT
);
- kfree(settings);
}
static int kobil_ioctl(struct tty_struct *tty, struct file *file,
--
1.6.6
On Mon, 28 Dec 2009 23:01:56 +0100
Johan Hovold <[email protected]> wrote:
[...]
> diff --git a/drivers/usb/serial/oti6858.c
> b/drivers/usb/serial/oti6858.c index c644e26..2ae97e7 100644
> --- a/drivers/usb/serial/oti6858.c
> +++ b/drivers/usb/serial/oti6858.c
> @@ -302,7 +302,7 @@ void send_data(struct work_struct *work)
> struct usb_serial_port *port = priv->port;
> int count = 0, result;
> unsigned long flags;
> - unsigned char allow;
> + u8 *allow;
>
> dbg("%s(port = %d)", __func__, port->number);
>
Erm, am I missing something here? You're removing a 1 byte
stack variable and replacing it with a 4 byte (pointer) stack
variable? What's the point of that, exactly?
On Mon, 28 Dec 2009 17:46:19 -0500
Andres Salomon <[email protected]> wrote:
> On Mon, 28 Dec 2009 23:01:56 +0100
> Johan Hovold <[email protected]> wrote:
>
> [...]
> > diff --git a/drivers/usb/serial/oti6858.c
> > b/drivers/usb/serial/oti6858.c index c644e26..2ae97e7 100644
> > --- a/drivers/usb/serial/oti6858.c
> > +++ b/drivers/usb/serial/oti6858.c
> > @@ -302,7 +302,7 @@ void send_data(struct work_struct *work)
> > struct usb_serial_port *port = priv->port;
> > int count = 0, result;
> > unsigned long flags;
> > - unsigned char allow;
> > + u8 *allow;
> >
> > dbg("%s(port = %d)", __func__, port->number);
> >
>
> Erm, am I missing something here? You're removing a 1 byte
> stack variable and replacing it with a 4 byte (pointer) stack
> variable? What's the point of that, exactly?
>
Ok, ignore that, I just figured it out (the usb_control_msg bit). :)
On Mon, Dec 28, 2009 at 11:01:44PM +0100, Johan Hovold wrote:
> Hi,
>
> These patches should address all remaining instances of DMA buffers on stack
> (being passed to usb_control_msg) for the usb serial drivers. They also fix a
> few endianess bugs I found along the way and do some minor clean up.
>
Wow. You have inspired me again to update smatch. It didn't catch any of
those.
Smatch found one other dma bug so I'll send a patch for it. (I would feel like
a jerk leaving it for you to fix on top of all the other stuff you have
done.)
regards,
dan carpenter
The original code did DMA on a stack variable, so I made it an allocated
pointer.
Compile tested only.
Signed-off-by: Dan Carpenter <[email protected]>
--- orig/drivers/usb/serial/io_edgeport.c 2009-12-30 16:20:35.000000000 +0200
+++ devel/drivers/usb/serial/io_edgeport.c 2009-12-30 16:25:27.000000000 +0200
@@ -372,21 +372,24 @@ static void update_edgeport_E2PROM(struc
************************************************************************/
static int get_string(struct usb_device *dev, int Id, char *string, int buflen)
{
- struct usb_string_descriptor StringDesc;
+ struct usb_string_descriptor *StringDesc;
struct usb_string_descriptor *pStringDesc;
dbg("%s - USB String ID = %d", __func__, Id);
+ StringDesc = kmalloc(sizeof(*StringDesc), GFP_KERNEL);
+ if (!StringDesc)
+ return 0;
if (!usb_get_descriptor(dev, USB_DT_STRING, Id,
- &StringDesc, sizeof(StringDesc)))
+ StringDesc, sizeof(*StringDesc)))
return 0;
- pStringDesc = kmalloc(StringDesc.bLength, GFP_KERNEL);
+ pStringDesc = kmalloc(StringDesc->bLength, GFP_KERNEL);
if (!pStringDesc)
return 0;
if (!usb_get_descriptor(dev, USB_DT_STRING, Id,
- pStringDesc, StringDesc.bLength)) {
+ pStringDesc, StringDesc->bLength)) {
kfree(pStringDesc);
return 0;
}
Hey Dan,
Looks like you forgot to free StringDesc:
> --- orig/drivers/usb/serial/io_edgeport.c 2009-12-30 16:20:35.000000000 +0200
> +++ devel/drivers/usb/serial/io_edgeport.c 2009-12-30 16:25:27.000000000 +0200
> @@ -372,21 +372,24 @@ static void update_edgeport_E2PROM(struc
> ************************************************************************/
> static int get_string(struct usb_device *dev, int Id, char *string, int buflen)
> {
> - struct usb_string_descriptor StringDesc;
> + struct usb_string_descriptor *StringDesc;
> struct usb_string_descriptor *pStringDesc;
>
> dbg("%s - USB String ID = %d", __func__, Id);
>
> + StringDesc = kmalloc(sizeof(*StringDesc), GFP_KERNEL);
> + if (!StringDesc)
> + return 0;
> if (!usb_get_descriptor(dev, USB_DT_STRING, Id,
> - &StringDesc, sizeof(StringDesc)))
> + StringDesc, sizeof(*StringDesc)))
here
> return 0;
>
> - pStringDesc = kmalloc(StringDesc.bLength, GFP_KERNEL);
> + pStringDesc = kmalloc(StringDesc->bLength, GFP_KERNEL);
> if (!pStringDesc)
here
> return 0;
>
> if (!usb_get_descriptor(dev, USB_DT_STRING, Id,
> - pStringDesc, StringDesc.bLength)) {
> + pStringDesc, StringDesc->bLength)) {
here
> kfree(pStringDesc);
> return 0;
> }
and here.
Thanks,
Johan
On Wed, Dec 30, 2009 at 06:06:11PM +0200, Dan Carpenter wrote:
> On Mon, Dec 28, 2009 at 11:01:44PM +0100, Johan Hovold wrote:
> > Hi,
> >
> > These patches should address all remaining instances of DMA buffers on stack
> > (being passed to usb_control_msg) for the usb serial drivers. They also fix a
> > few endianess bugs I found along the way and do some minor clean up.
> >
>
> Wow. You have inspired me again to update smatch. It didn't catch any of
> those.
>
> Smatch found one other dma bug so I'll send a patch for it. (I would feel like
> a jerk leaving it for you to fix on top of all the other stuff you have
> done.)
Good catch. I only backtracked from direct calls to usb_control_msg. :)
Doing so by hand can be quite tedious so automatic testing for this is
greatly appreciated.
Thanks,
Johan
On Wed, Dec 30, 2009 at 06:14:58PM +0100, Johan Hovold wrote:
> Hey Dan,
>
> Looks like you forgot to free StringDesc:
>
Gar... That's embarrassing. I'm really sorry about that.
I will send an updated patch tomorrow.
Also smatch sort of should have caught that with the --spammy option but
it didn't. :/ In general the leak checks are pretty terrible. Will
look at this as well.
regards,
dan carpenter
Please disregard this one as I just realised that this has already been
fixed by Pete Zaitcev (in Greg's patch tree). The patches basically only
differs in that I added error messages for failed allocations, and
that Pete used put_unaligned for the divsor.
Sorry about that.
/Johan
The original code was passing a stack variable as a dma buffer, so I
made it an allocated variable. Instead of adding a bunch of kfree()
calls, I changed all the error return paths to gotos.
Also I noticed that the error checking wasn't correct because
usb_get_descriptor() can return negative values.
While I was at it, I made an unrelated white space change by moving
the unicode_to_ascii() on to one line.
Signed-off-by: Dan Carpenter <[email protected]>
--- orig/drivers/usb/serial/io_edgeport.c 2009-12-30 16:20:35.000000000 +0200
+++ devel/drivers/usb/serial/io_edgeport.c 2009-12-30 23:48:53.000000000 +0200
@@ -372,31 +372,32 @@ static void update_edgeport_E2PROM(struc
************************************************************************/
static int get_string(struct usb_device *dev, int Id, char *string, int buflen)
{
- struct usb_string_descriptor StringDesc;
- struct usb_string_descriptor *pStringDesc;
+ struct usb_string_descriptor *StringDesc = NULL;
+ struct usb_string_descriptor *pStringDesc = NULL;
+ int ret = 0;
dbg("%s - USB String ID = %d", __func__, Id);
- if (!usb_get_descriptor(dev, USB_DT_STRING, Id,
- &StringDesc, sizeof(StringDesc)))
- return 0;
+ StringDesc = kmalloc(sizeof(*StringDesc), GFP_KERNEL);
+ if (!StringDesc)
+ goto free;
+ if (usb_get_descriptor(dev, USB_DT_STRING, Id, StringDesc, sizeof(*StringDesc)) <= 0)
+ goto free;
- pStringDesc = kmalloc(StringDesc.bLength, GFP_KERNEL);
+ pStringDesc = kmalloc(StringDesc->bLength, GFP_KERNEL);
if (!pStringDesc)
- return 0;
+ goto free;
- if (!usb_get_descriptor(dev, USB_DT_STRING, Id,
- pStringDesc, StringDesc.bLength)) {
- kfree(pStringDesc);
- return 0;
- }
-
- unicode_to_ascii(string, buflen,
- pStringDesc->wData, pStringDesc->bLength/2);
+ if (usb_get_descriptor(dev, USB_DT_STRING, Id, pStringDesc, StringDesc->bLength) <= 0)
+ goto free;
- kfree(pStringDesc);
+ unicode_to_ascii(string, buflen, pStringDesc->wData, pStringDesc->bLength/2);
+ ret = strlen(string);
dbg("%s - USB String %s", __func__, string);
- return strlen(string);
+free:
+ kfree(StringDesc);
+ kfree(pStringDesc);
+ return ret;
}
Hi,
Here's an updated series rebased against rc2 with all patches from Greg's tree
applied. The mct_u232 patch has been dropped as it had already been fixed. I
also updated the ch341 and cypress_m8 patches to use the unaligned macros.
Johan
Johan Hovold (13):
USB: ch341: replace printk warnings with dev_err
USB: ch341: fix DMA buffer on stack
USB: ch341: use get_unaligned_le16 in break_ctl
USB: cypress_m8: fix DMA buffer on stack
USB: cypress_m8: fix endianess bug and alignment
USB: io_ti: fix DMA buffers on stack
USB: keyspan_pda: fix DMA buffers on stack
USB: kl5kusb105: fix DMA buffers on stack
USB: mos7720: fix DMA buffers on stack and clean up send_mos_cmd
USB: mos7840: fix DMA buffers on stack and endianess bugs
USB: oti6858: fix DMA buffer on stack
USB: visor: fix DMA buffers on stack
USB: kobil_sct: clean up kobil_set_termios
drivers/usb/serial/ch341.c | 25 ++++++++++----
drivers/usb/serial/cypress_m8.c | 36 +++++++++++---------
drivers/usb/serial/io_ti.c | 66 +++++++++++++++++++++++++++----------
drivers/usb/serial/keyspan_pda.c | 45 +++++++++++++++++++-------
drivers/usb/serial/kl5kusb105.c | 63 ++++++++++++++++++++++++++----------
drivers/usb/serial/kobil_sct.c | 22 ++----------
drivers/usb/serial/mos7720.c | 34 ++++++++++----------
drivers/usb/serial/mos7840.c | 22 ++++++++++--
drivers/usb/serial/oti6858.c | 13 ++++++--
drivers/usb/serial/visor.c | 26 ++++++++++----
10 files changed, 230 insertions(+), 122 deletions(-)
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/ch341.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 59eff72..5e455f4 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -399,8 +399,8 @@ static void ch341_break_ctl(struct tty_struct *tty, int break_state)
r = ch341_control_in(port->serial->dev, CH341_REQ_READ_REG,
ch341_break_reg, 0, break_reg, sizeof(break_reg));
if (r < 0) {
- printk(KERN_WARNING "%s: USB control read error whilst getting"
- " break register contents.\n", __FILE__);
+ dev_err(&port->dev, "%s - USB control read error (%d)\n",
+ __func__, r);
return;
}
dbg("%s - initial ch341 break register contents - reg1: %x, reg2: %x",
@@ -420,8 +420,8 @@ static void ch341_break_ctl(struct tty_struct *tty, int break_state)
r = ch341_control_out(port->serial->dev, CH341_REQ_WRITE_REG,
ch341_break_reg, reg_contents);
if (r < 0)
- printk(KERN_WARNING "%s: USB control write error whilst setting"
- " break register contents.\n", __FILE__);
+ dev_err(&port->dev, "%s - USB control write error (%d)\n",
+ __func__, r);
}
static int ch341_tiocmset(struct tty_struct *tty, struct file *file,
--
1.6.6
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/ch341.c | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 5e455f4..2942c6d 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -392,16 +392,22 @@ static void ch341_break_ctl(struct tty_struct *tty, int break_state)
struct usb_serial_port *port = tty->driver_data;
int r;
uint16_t reg_contents;
- uint8_t break_reg[2];
+ uint8_t *break_reg;
dbg("%s()", __func__);
+ break_reg = kmalloc(2, GFP_KERNEL);
+ if (!break_reg) {
+ dev_err(&port->dev, "%s - kmalloc failed\n", __func__);
+ return;
+ }
+
r = ch341_control_in(port->serial->dev, CH341_REQ_READ_REG,
- ch341_break_reg, 0, break_reg, sizeof(break_reg));
+ ch341_break_reg, 0, break_reg, 2);
if (r < 0) {
dev_err(&port->dev, "%s - USB control read error (%d)\n",
__func__, r);
- return;
+ goto out;
}
dbg("%s - initial ch341 break register contents - reg1: %x, reg2: %x",
__func__, break_reg[0], break_reg[1]);
@@ -422,6 +428,8 @@ static void ch341_break_ctl(struct tty_struct *tty, int break_state)
if (r < 0)
dev_err(&port->dev, "%s - USB control write error (%d)\n",
__func__, r);
+out:
+ kfree(break_reg);
}
static int ch341_tiocmset(struct tty_struct *tty, struct file *file,
--
1.6.6
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/ch341.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 2942c6d..6230d24 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -22,6 +22,7 @@
#include <linux/usb.h>
#include <linux/usb/serial.h>
#include <linux/serial.h>
+#include <asm/unaligned.h>
#define DEFAULT_BAUD_RATE 9600
#define DEFAULT_TIMEOUT 1000
@@ -422,7 +423,7 @@ static void ch341_break_ctl(struct tty_struct *tty, int break_state)
}
dbg("%s - New ch341 break register contents - reg1: %x, reg2: %x",
__func__, break_reg[0], break_reg[1]);
- reg_contents = (uint16_t)break_reg[0] | ((uint16_t)break_reg[1] << 8);
+ reg_contents = get_unaligned_le16(break_reg);
r = ch341_control_out(port->serial->dev, CH341_REQ_WRITE_REG,
ch341_break_reg, reg_contents);
if (r < 0)
--
1.6.6
Cc: Lonnie Mendez <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/cypress_m8.c | 31 +++++++++++++++++--------------
1 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c
index 60c2002..1ce1a3a 100644
--- a/drivers/usb/serial/cypress_m8.c
+++ b/drivers/usb/serial/cypress_m8.c
@@ -344,7 +344,8 @@ static int cypress_serial_control(struct tty_struct *tty,
{
int new_baudrate = 0, retval = 0, tries = 0;
struct cypress_private *priv;
- __u8 feature_buffer[5];
+ u8 *feature_buffer;
+ const unsigned int feature_len = 5;
unsigned long flags;
dbg("%s", __func__);
@@ -354,6 +355,10 @@ static int cypress_serial_control(struct tty_struct *tty,
if (!priv->comm_is_ok)
return -ENODEV;
+ feature_buffer = kcalloc(feature_len, sizeof(u8), GFP_KERNEL);
+ if (!feature_buffer)
+ return -ENOMEM;
+
switch (cypress_request_type) {
case CYPRESS_SET_CONFIG:
/* 0 means 'Hang up' so doesn't change the true bit rate */
@@ -370,7 +375,6 @@ static int cypress_serial_control(struct tty_struct *tty,
dbg("%s - baud rate is being sent as %d",
__func__, new_baudrate);
- memset(feature_buffer, 0, sizeof(feature_buffer));
/* fill the feature_buffer with new configuration */
*((u_int32_t *)feature_buffer) = new_baudrate;
feature_buffer[4] |= data_bits; /* assign data bits in 2 bit space ( max 3 ) */
@@ -394,15 +398,15 @@ static int cypress_serial_control(struct tty_struct *tty,
HID_REQ_SET_REPORT,
USB_DIR_OUT | USB_RECIP_INTERFACE | USB_TYPE_CLASS,
0x0300, 0, feature_buffer,
- sizeof(feature_buffer), 500);
+ feature_len, 500);
if (tries++ >= 3)
break;
- } while (retval != sizeof(feature_buffer) &&
+ } while (retval != feature_len &&
retval != -ENODEV);
- if (retval != sizeof(feature_buffer)) {
+ if (retval != feature_len) {
dev_err(&port->dev, "%s - failed sending serial "
"line settings - %d\n", __func__, retval);
cypress_set_dead(port);
@@ -422,30 +426,28 @@ static int cypress_serial_control(struct tty_struct *tty,
/* Not implemented for this device,
and if we try to do it we're likely
to crash the hardware. */
- return -ENOTTY;
+ retval = -ENOTTY;
+ goto out;
}
dbg("%s - retreiving serial line settings", __func__);
- /* set initial values in feature buffer */
- memset(feature_buffer, 0, sizeof(feature_buffer));
-
do {
retval = usb_control_msg(port->serial->dev,
usb_rcvctrlpipe(port->serial->dev, 0),
HID_REQ_GET_REPORT,
USB_DIR_IN | USB_RECIP_INTERFACE | USB_TYPE_CLASS,
0x0300, 0, feature_buffer,
- sizeof(feature_buffer), 500);
+ feature_len, 500);
if (tries++ >= 3)
break;
- } while (retval != sizeof(feature_buffer)
+ } while (retval != feature_len
&& retval != -ENODEV);
- if (retval != sizeof(feature_buffer)) {
+ if (retval != feature_len) {
dev_err(&port->dev, "%s - failed to retrieve serial "
"line settings - %d\n", __func__, retval);
cypress_set_dead(port);
- return retval;
+ goto out;
} else {
spin_lock_irqsave(&priv->lock, flags);
/* store the config in one byte, and later
@@ -458,7 +460,8 @@ static int cypress_serial_control(struct tty_struct *tty,
spin_lock_irqsave(&priv->lock, flags);
++priv->cmd_count;
spin_unlock_irqrestore(&priv->lock, flags);
-
+out:
+ kfree(feature_buffer);
return retval;
} /* cypress_serial_control */
--
1.6.6
Cc: Lonnie Mendez <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/cypress_m8.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c
index 1ce1a3a..b19e16a 100644
--- a/drivers/usb/serial/cypress_m8.c
+++ b/drivers/usb/serial/cypress_m8.c
@@ -66,6 +66,7 @@
#include <linux/serial.h>
#include <linux/delay.h>
#include <linux/uaccess.h>
+#include <asm/unaligned.h>
#include "cypress_m8.h"
@@ -376,7 +377,7 @@ static int cypress_serial_control(struct tty_struct *tty,
__func__, new_baudrate);
/* fill the feature_buffer with new configuration */
- *((u_int32_t *)feature_buffer) = new_baudrate;
+ put_unaligned_le32(new_baudrate, feature_buffer);
feature_buffer[4] |= data_bits; /* assign data bits in 2 bit space ( max 3 ) */
/* 1 bit gap */
feature_buffer[4] |= (stop_bits << 3); /* assign stop bits in 1 bit space */
@@ -453,7 +454,7 @@ static int cypress_serial_control(struct tty_struct *tty,
/* store the config in one byte, and later
use bit masks to check values */
priv->current_config = feature_buffer[4];
- priv->baud_rate = *((u_int32_t *)feature_buffer);
+ priv->baud_rate = get_unaligned_le32(feature_buffer);
spin_unlock_irqrestore(&priv->lock, flags);
}
}
--
1.6.6
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/io_ti.c | 66 ++++++++++++++++++++++++++++++++------------
1 files changed, 48 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index d4cc0f7..1691f07 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -413,11 +413,18 @@ static int write_boot_mem(struct edgeport_serial *serial,
{
int status = 0;
int i;
- __u8 temp;
+ u8 *temp;
/* Must do a read before write */
if (!serial->TiReadI2C) {
- status = read_boot_mem(serial, 0, 1, &temp);
+ temp = kmalloc(1, GFP_KERNEL);
+ if (!temp) {
+ dev_err(&serial->serial->dev->dev,
+ "%s - out of memory\n", __func__);
+ return -ENOMEM;
+ }
+ status = read_boot_mem(serial, 0, 1, temp);
+ kfree(temp);
if (status)
return status;
}
@@ -935,37 +942,47 @@ static int build_i2c_fw_hdr(__u8 *header, struct device *dev)
static int i2c_type_bootmode(struct edgeport_serial *serial)
{
int status;
- __u8 data;
+ u8 *data;
+
+ data = kmalloc(1, GFP_KERNEL);
+ if (!data) {
+ dev_err(&serial->serial->dev->dev,
+ "%s - out of memory\n", __func__);
+ return -ENOMEM;
+ }
/* Try to read type 2 */
status = ti_vread_sync(serial->serial->dev, UMPC_MEMORY_READ,
- DTK_ADDR_SPACE_I2C_TYPE_II, 0, &data, 0x01);
+ DTK_ADDR_SPACE_I2C_TYPE_II, 0, data, 0x01);
if (status)
dbg("%s - read 2 status error = %d", __func__, status);
else
- dbg("%s - read 2 data = 0x%x", __func__, data);
- if ((!status) && (data == UMP5152 || data == UMP3410)) {
+ dbg("%s - read 2 data = 0x%x", __func__, *data);
+ if ((!status) && (*data == UMP5152 || *data == UMP3410)) {
dbg("%s - ROM_TYPE_II", __func__);
serial->TI_I2C_Type = DTK_ADDR_SPACE_I2C_TYPE_II;
- return 0;
+ goto out;
}
/* Try to read type 3 */
status = ti_vread_sync(serial->serial->dev, UMPC_MEMORY_READ,
- DTK_ADDR_SPACE_I2C_TYPE_III, 0, &data, 0x01);
+ DTK_ADDR_SPACE_I2C_TYPE_III, 0, data, 0x01);
if (status)
dbg("%s - read 3 status error = %d", __func__, status);
else
- dbg("%s - read 2 data = 0x%x", __func__, data);
- if ((!status) && (data == UMP5152 || data == UMP3410)) {
+ dbg("%s - read 2 data = 0x%x", __func__, *data);
+ if ((!status) && (*data == UMP5152 || *data == UMP3410)) {
dbg("%s - ROM_TYPE_III", __func__);
serial->TI_I2C_Type = DTK_ADDR_SPACE_I2C_TYPE_III;
- return 0;
+ goto out;
}
dbg("%s - Unknown", __func__);
serial->TI_I2C_Type = DTK_ADDR_SPACE_I2C_TYPE_II;
- return -ENODEV;
+ status = -ENODEV;
+out:
+ kfree(data);
+ return status;
}
static int bulk_xfer(struct usb_serial *serial, void *buffer,
@@ -1113,7 +1130,7 @@ static int download_fw(struct edgeport_serial *serial)
I2C_DESC_TYPE_FIRMWARE_BASIC, rom_desc);
if (start_address != 0) {
struct ti_i2c_firmware_rec *firmware_version;
- __u8 record;
+ u8 *record;
dbg("%s - Found Type FIRMWARE (Type 2) record",
__func__);
@@ -1165,6 +1182,15 @@ static int download_fw(struct edgeport_serial *serial)
OperationalMajorVersion,
OperationalMinorVersion);
+ record = kmalloc(1, GFP_KERNEL);
+ if (!record) {
+ dev_err(dev, "%s - out of memory.\n",
+ __func__);
+ kfree(firmware_version);
+ kfree(rom_desc);
+ kfree(ti_manuf_desc);
+ return -ENOMEM;
+ }
/* In order to update the I2C firmware we must
* change the type 2 record to type 0xF2. This
* will force the UMP to come up in Boot Mode.
@@ -1177,13 +1203,14 @@ static int download_fw(struct edgeport_serial *serial)
* firmware will update the record type from
* 0xf2 to 0x02.
*/
- record = I2C_DESC_TYPE_FIRMWARE_BLANK;
+ *record = I2C_DESC_TYPE_FIRMWARE_BLANK;
/* Change the I2C Firmware record type to
0xf2 to trigger an update */
status = write_rom(serial, start_address,
- sizeof(record), &record);
+ sizeof(*record), record);
if (status) {
+ kfree(record);
kfree(firmware_version);
kfree(rom_desc);
kfree(ti_manuf_desc);
@@ -1196,19 +1223,21 @@ static int download_fw(struct edgeport_serial *serial)
*/
status = read_rom(serial,
start_address,
- sizeof(record),
- &record);
+ sizeof(*record),
+ record);
if (status) {
+ kfree(record);
kfree(firmware_version);
kfree(rom_desc);
kfree(ti_manuf_desc);
return status;
}
- if (record != I2C_DESC_TYPE_FIRMWARE_BLANK) {
+ if (*record != I2C_DESC_TYPE_FIRMWARE_BLANK) {
dev_err(dev,
"%s - error resetting device\n",
__func__);
+ kfree(record);
kfree(firmware_version);
kfree(rom_desc);
kfree(ti_manuf_desc);
@@ -1226,6 +1255,7 @@ static int download_fw(struct edgeport_serial *serial)
__func__, status);
/* return an error on purpose. */
+ kfree(record);
kfree(firmware_version);
kfree(rom_desc);
kfree(ti_manuf_desc);
--
1.6.6
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/keyspan_pda.c | 45 +++++++++++++++++++++++++++----------
1 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c
index 1296a09..427d377 100644
--- a/drivers/usb/serial/keyspan_pda.c
+++ b/drivers/usb/serial/keyspan_pda.c
@@ -429,13 +429,20 @@ static int keyspan_pda_get_modem_info(struct usb_serial *serial,
unsigned char *value)
{
int rc;
- unsigned char data;
+ u8 *data;
+
+ data = kmalloc(1, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
rc = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
3, /* get pins */
USB_TYPE_VENDOR|USB_RECIP_INTERFACE|USB_DIR_IN,
- 0, 0, &data, 1, 2000);
+ 0, 0, data, 1, 2000);
if (rc >= 0)
- *value = data;
+ *value = *data;
+
+ kfree(data);
return rc;
}
@@ -543,7 +550,14 @@ static int keyspan_pda_write(struct tty_struct *tty,
device how much room it really has. This is done only on
scheduler time, since usb_control_msg() sleeps. */
if (count > priv->tx_room && !in_interrupt()) {
- unsigned char room;
+ u8 *room;
+
+ room = kmalloc(1, GFP_KERNEL);
+ if (!room) {
+ rc = -ENOMEM;
+ goto exit;
+ }
+
rc = usb_control_msg(serial->dev,
usb_rcvctrlpipe(serial->dev, 0),
6, /* write_room */
@@ -551,9 +565,14 @@ static int keyspan_pda_write(struct tty_struct *tty,
| USB_DIR_IN,
0, /* value: 0 means "remaining room" */
0, /* index */
- &room,
+ room,
1,
2000);
+ if (rc > 0) {
+ dbg(" roomquery says %d", *room);
+ priv->tx_room = *room;
+ }
+ kfree(room);
if (rc < 0) {
dbg(" roomquery failed");
goto exit;
@@ -563,8 +582,6 @@ static int keyspan_pda_write(struct tty_struct *tty,
rc = -EIO; /* device didn't return any data */
goto exit;
}
- dbg(" roomquery says %d", room);
- priv->tx_room = room;
}
if (count > priv->tx_room) {
/* we're about to completely fill the Tx buffer, so
@@ -684,18 +701,22 @@ static int keyspan_pda_open(struct tty_struct *tty,
struct usb_serial_port *port)
{
struct usb_serial *serial = port->serial;
- unsigned char room;
+ u8 *room;
int rc = 0;
struct keyspan_pda_private *priv;
/* find out how much room is in the Tx ring */
+ room = kmalloc(1, GFP_KERNEL);
+ if (!room)
+ return -ENOMEM;
+
rc = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
6, /* write_room */
USB_TYPE_VENDOR | USB_RECIP_INTERFACE
| USB_DIR_IN,
0, /* value */
0, /* index */
- &room,
+ room,
1,
2000);
if (rc < 0) {
@@ -708,8 +729,8 @@ static int keyspan_pda_open(struct tty_struct *tty,
goto error;
}
priv = usb_get_serial_port_data(port);
- priv->tx_room = room;
- priv->tx_throttled = room ? 0 : 1;
+ priv->tx_room = *room;
+ priv->tx_throttled = *room ? 0 : 1;
/*Start reading from the device*/
port->interrupt_in_urb->dev = serial->dev;
@@ -718,8 +739,8 @@ static int keyspan_pda_open(struct tty_struct *tty,
dbg("%s - usb_submit_urb(read int) failed", __func__);
goto error;
}
-
error:
+ kfree(room);
return rc;
}
static void keyspan_pda_close(struct usb_serial_port *port)
--
1.6.6
Cc: Oliver Neukum <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/kl5kusb105.c | 63 +++++++++++++++++++++++++++-----------
1 files changed, 45 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/serial/kl5kusb105.c b/drivers/usb/serial/kl5kusb105.c
index 3a78738..731964b 100644
--- a/drivers/usb/serial/kl5kusb105.c
+++ b/drivers/usb/serial/kl5kusb105.c
@@ -212,10 +212,19 @@ static int klsi_105_get_line_state(struct usb_serial_port *port,
unsigned long *line_state_p)
{
int rc;
- __u8 status_buf[KLSI_STATUSBUF_LEN] = { -1, -1};
+ u8 *status_buf;
__u16 status;
dev_info(&port->serial->dev->dev, "sending SIO Poll request\n");
+
+ status_buf = kmalloc(KLSI_STATUSBUF_LEN, GFP_KERNEL);
+ if (!status_buf) {
+ dev_err(&port->dev, "%s - out of memory for status buffer.\n",
+ __func__);
+ return -ENOMEM;
+ }
+ status_buf[0] = 0xff;
+ status_buf[1] = 0xff;
rc = usb_control_msg(port->serial->dev,
usb_rcvctrlpipe(port->serial->dev, 0),
KL5KUSB105A_SIO_POLL,
@@ -236,6 +245,8 @@ static int klsi_105_get_line_state(struct usb_serial_port *port,
*line_state_p = klsi_105_status2linestate(status);
}
+
+ kfree(status_buf);
return rc;
}
@@ -364,7 +375,7 @@ static int klsi_105_open(struct tty_struct *tty, struct usb_serial_port *port)
int rc;
int i;
unsigned long line_state;
- struct klsi_105_port_settings cfg;
+ struct klsi_105_port_settings *cfg;
unsigned long flags;
dbg("%s port %d", __func__, port->number);
@@ -376,12 +387,18 @@ static int klsi_105_open(struct tty_struct *tty, struct usb_serial_port *port)
* Then read the modem line control and store values in
* priv->line_state.
*/
- cfg.pktlen = 5;
- cfg.baudrate = kl5kusb105a_sio_b9600;
- cfg.databits = kl5kusb105a_dtb_8;
- cfg.unknown1 = 0;
- cfg.unknown2 = 1;
- klsi_105_chg_port_settings(port, &cfg);
+ cfg = kmalloc(sizeof(*cfg), GFP_KERNEL);
+ if (!cfg) {
+ dev_err(&port->dev, "%s - out of memory for config buffer.\n",
+ __func__);
+ return -ENOMEM;
+ }
+ cfg->pktlen = 5;
+ cfg->baudrate = kl5kusb105a_sio_b9600;
+ cfg->databits = kl5kusb105a_dtb_8;
+ cfg->unknown1 = 0;
+ cfg->unknown2 = 1;
+ klsi_105_chg_port_settings(port, cfg);
/* set up termios structure */
spin_lock_irqsave(&priv->lock, flags);
@@ -391,11 +408,11 @@ static int klsi_105_open(struct tty_struct *tty, struct usb_serial_port *port)
priv->termios.c_lflag = tty->termios->c_lflag;
for (i = 0; i < NCCS; i++)
priv->termios.c_cc[i] = tty->termios->c_cc[i];
- priv->cfg.pktlen = cfg.pktlen;
- priv->cfg.baudrate = cfg.baudrate;
- priv->cfg.databits = cfg.databits;
- priv->cfg.unknown1 = cfg.unknown1;
- priv->cfg.unknown2 = cfg.unknown2;
+ priv->cfg.pktlen = cfg->pktlen;
+ priv->cfg.baudrate = cfg->baudrate;
+ priv->cfg.databits = cfg->databits;
+ priv->cfg.unknown1 = cfg->unknown1;
+ priv->cfg.unknown2 = cfg->unknown2;
spin_unlock_irqrestore(&priv->lock, flags);
/* READ_ON and urb submission */
@@ -441,6 +458,7 @@ static int klsi_105_open(struct tty_struct *tty, struct usb_serial_port *port)
retval = rc;
exit:
+ kfree(cfg);
return retval;
} /* klsi_105_open */
@@ -714,10 +732,17 @@ static void klsi_105_set_termios(struct tty_struct *tty,
unsigned int old_iflag = old_termios->c_iflag;
unsigned int cflag = tty->termios->c_cflag;
unsigned int old_cflag = old_termios->c_cflag;
- struct klsi_105_port_settings cfg;
+ struct klsi_105_port_settings *cfg;
unsigned long flags;
speed_t baud;
+ cfg = kmalloc(sizeof(*cfg), GFP_KERNEL);
+ if (!cfg) {
+ dev_err(&port->dev, "%s - out of memory for config buffer.\n",
+ __func__);
+ return;
+ }
+
/* lock while we are modifying the settings */
spin_lock_irqsave(&priv->lock, flags);
@@ -793,11 +818,11 @@ static void klsi_105_set_termios(struct tty_struct *tty,
case CS5:
dbg("%s - 5 bits/byte not supported", __func__);
spin_unlock_irqrestore(&priv->lock, flags);
- return ;
+ goto err;
case CS6:
dbg("%s - 6 bits/byte not supported", __func__);
spin_unlock_irqrestore(&priv->lock, flags);
- return ;
+ goto err;
case CS7:
priv->cfg.databits = kl5kusb105a_dtb_7;
break;
@@ -856,11 +881,13 @@ static void klsi_105_set_termios(struct tty_struct *tty,
#endif
;
}
- memcpy(&cfg, &priv->cfg, sizeof(cfg));
+ memcpy(cfg, &priv->cfg, sizeof(*cfg));
spin_unlock_irqrestore(&priv->lock, flags);
/* now commit changes to device */
- klsi_105_chg_port_settings(port, &cfg);
+ klsi_105_chg_port_settings(port, cfg);
+err:
+ kfree(cfg);
} /* klsi_105_set_termios */
--
1.6.6
Change data-argument type from (void *) to (u8 *) to prevent endianess
problems.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/mos7720.c | 34 +++++++++++++++++-----------------
1 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
index 763e32a..e081dc0 100644
--- a/drivers/usb/serial/mos7720.c
+++ b/drivers/usb/serial/mos7720.c
@@ -275,13 +275,11 @@ static void mos7720_bulk_out_data_callback(struct urb *urb)
* this function will be used for sending command to device
*/
static int send_mos_cmd(struct usb_serial *serial, __u8 request, __u16 value,
- __u16 index, void *data)
+ __u16 index, u8 *data)
{
int status;
- unsigned int pipe;
+ u8 *buf;
u16 product = le16_to_cpu(serial->dev->descriptor.idProduct);
- __u8 requesttype;
- __u16 size = 0x0000;
if (value < MOS_MAX_PORT) {
if (product == MOSCHIP_DEVICE_ID_7715)
@@ -298,21 +296,23 @@ static int send_mos_cmd(struct usb_serial *serial, __u8 request, __u16 value,
}
if (request == MOS_WRITE) {
- request = (__u8)MOS_WRITE;
- requesttype = (__u8)0x40;
- value = value + (__u16)*((unsigned char *)data);
- data = NULL;
- pipe = usb_sndctrlpipe(serial->dev, 0);
+ value = value + *data;
+ status = usb_control_msg(serial->dev,
+ usb_sndctrlpipe(serial->dev, 0), MOS_WRITE,
+ 0x40, value, index, NULL, 0, MOS_WDR_TIMEOUT);
} else {
- request = (__u8)MOS_READ;
- requesttype = (__u8)0xC0;
- size = 0x01;
- pipe = usb_rcvctrlpipe(serial->dev, 0);
+ buf = kmalloc(1, GFP_KERNEL);
+ if (!buf) {
+ status = -ENOMEM;
+ goto out;
+ }
+ status = usb_control_msg(serial->dev,
+ usb_rcvctrlpipe(serial->dev, 0), MOS_READ,
+ 0xc0, value, index, buf, 1, MOS_WDR_TIMEOUT);
+ *data = *buf;
+ kfree(buf);
}
-
- status = usb_control_msg(serial->dev, pipe, request, requesttype,
- value, index, data, size, MOS_WDR_TIMEOUT);
-
+out:
if (status < 0)
dbg("Command Write failed Value %x index %x\n", value, index);
--
1.6.6
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/mos7840.c | 22 ++++++++++++++++++----
1 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 2cfe245..04bef4b 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -283,12 +283,19 @@ static int mos7840_get_reg_sync(struct usb_serial_port *port, __u16 reg,
{
struct usb_device *dev = port->serial->dev;
int ret = 0;
+ u8 *buf;
+
+ buf = kmalloc(VENDOR_READ_LENGTH, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), MCS_RDREQ,
- MCS_RD_RTYPE, 0, reg, val, VENDOR_READ_LENGTH,
+ MCS_RD_RTYPE, 0, reg, buf, VENDOR_READ_LENGTH,
MOS_WDR_TIMEOUT);
+ *val = buf[0];
dbg("mos7840_get_reg_sync offset is %x, return val %x", reg, *val);
- *val = (*val) & 0x00ff;
+
+ kfree(buf);
return ret;
}
@@ -341,6 +348,11 @@ static int mos7840_get_uart_reg(struct usb_serial_port *port, __u16 reg,
struct usb_device *dev = port->serial->dev;
int ret = 0;
__u16 Wval;
+ u8 *buf;
+
+ buf = kmalloc(VENDOR_READ_LENGTH, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
/* dbg("application number is %4x",
(((__u16)port->number - (__u16)(port->serial->minor))+1)<<8); */
@@ -364,9 +376,11 @@ static int mos7840_get_uart_reg(struct usb_serial_port *port, __u16 reg,
}
}
ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), MCS_RDREQ,
- MCS_RD_RTYPE, Wval, reg, val, VENDOR_READ_LENGTH,
+ MCS_RD_RTYPE, Wval, reg, buf, VENDOR_READ_LENGTH,
MOS_WDR_TIMEOUT);
- *val = (*val) & 0x00ff;
+ *val = buf[0];
+
+ kfree(buf);
return ret;
}
--
1.6.6
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/oti6858.c | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/serial/oti6858.c b/drivers/usb/serial/oti6858.c
index c644e26..2ae97e7 100644
--- a/drivers/usb/serial/oti6858.c
+++ b/drivers/usb/serial/oti6858.c
@@ -302,7 +302,7 @@ void send_data(struct work_struct *work)
struct usb_serial_port *port = priv->port;
int count = 0, result;
unsigned long flags;
- unsigned char allow;
+ u8 *allow;
dbg("%s(port = %d)", __func__, port->number);
@@ -321,13 +321,20 @@ void send_data(struct work_struct *work)
count = port->bulk_out_size;
if (count != 0) {
+ allow = kmalloc(1, GFP_KERNEL);
+ if (!allow) {
+ dev_err(&port->dev, "%s(): kmalloc failed\n",
+ __func__);
+ return;
+ }
result = usb_control_msg(port->serial->dev,
usb_rcvctrlpipe(port->serial->dev, 0),
OTI6858_REQ_T_CHECK_TXBUFF,
OTI6858_REQ_CHECK_TXBUFF,
- count, 0, &allow, 1, 100);
- if (result != 1 || allow != 0)
+ count, 0, allow, 1, 100);
+ if (result != 1 || *allow != 0)
count = 0;
+ kfree(allow);
}
if (count == 0) {
--
1.6.6
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/visor.c | 26 ++++++++++++++++++--------
1 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/serial/visor.c b/drivers/usb/serial/visor.c
index ad1f923..178e4d9 100644
--- a/drivers/usb/serial/visor.c
+++ b/drivers/usb/serial/visor.c
@@ -807,10 +807,14 @@ static int clie_3_5_startup(struct usb_serial *serial)
{
struct device *dev = &serial->dev->dev;
int result;
- u8 data;
+ u8 *data;
dbg("%s", __func__);
+ data = kmalloc(1, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
/*
* Note that PEG-300 series devices expect the following two calls.
*/
@@ -818,36 +822,42 @@ static int clie_3_5_startup(struct usb_serial *serial)
/* get the config number */
result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
USB_REQ_GET_CONFIGURATION, USB_DIR_IN,
- 0, 0, &data, 1, 3000);
+ 0, 0, data, 1, 3000);
if (result < 0) {
dev_err(dev, "%s: get config number failed: %d\n",
__func__, result);
- return result;
+ goto out;
}
if (result != 1) {
dev_err(dev, "%s: get config number bad return length: %d\n",
__func__, result);
- return -EIO;
+ result = -EIO;
+ goto out;
}
/* get the interface number */
result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
USB_REQ_GET_INTERFACE,
USB_DIR_IN | USB_RECIP_INTERFACE,
- 0, 0, &data, 1, 3000);
+ 0, 0, data, 1, 3000);
if (result < 0) {
dev_err(dev, "%s: get interface number failed: %d\n",
__func__, result);
- return result;
+ goto out;
}
if (result != 1) {
dev_err(dev,
"%s: get interface number bad return length: %d\n",
__func__, result);
- return -EIO;
+ result = -EIO;
+ goto out;
}
- return generic_startup(serial);
+ result = generic_startup(serial);
+out:
+ kfree(data);
+
+ return result;
}
static int treo_attach(struct usb_serial *serial)
--
1.6.6
Kill string that is allocated and generated using speed and parity
settings but is never used (and never has been).
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/kobil_sct.c | 22 ++++------------------
1 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/serial/kobil_sct.c b/drivers/usb/serial/kobil_sct.c
index 45ea694..f917c5b 100644
--- a/drivers/usb/serial/kobil_sct.c
+++ b/drivers/usb/serial/kobil_sct.c
@@ -624,7 +624,6 @@ static void kobil_set_termios(struct tty_struct *tty,
unsigned short urb_val = 0;
int c_cflag = tty->termios->c_cflag;
speed_t speed;
- void *settings;
priv = usb_get_serial_port_data(port);
if (priv->device_type == KOBIL_USBTWIN_PRODUCT_ID ||
@@ -647,25 +646,13 @@ static void kobil_set_termios(struct tty_struct *tty,
}
urb_val |= (c_cflag & CSTOPB) ? SUSBCR_SPASB_2StopBits :
SUSBCR_SPASB_1StopBit;
-
- settings = kzalloc(50, GFP_KERNEL);
- if (!settings)
- return;
-
- sprintf(settings, "%d ", speed);
-
if (c_cflag & PARENB) {
- if (c_cflag & PARODD) {
+ if (c_cflag & PARODD)
urb_val |= SUSBCR_SPASB_OddParity;
- strcat(settings, "Odd Parity");
- } else {
+ else
urb_val |= SUSBCR_SPASB_EvenParity;
- strcat(settings, "Even Parity");
- }
- } else {
+ } else
urb_val |= SUSBCR_SPASB_NoParity;
- strcat(settings, "No Parity");
- }
tty->termios->c_cflag &= ~CMSPAR;
tty_encode_baud_rate(tty, speed, speed);
@@ -675,11 +662,10 @@ static void kobil_set_termios(struct tty_struct *tty,
USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT,
urb_val,
0,
- settings,
+ NULL,
0,
KOBIL_TIMEOUT
);
- kfree(settings);
}
static int kobil_ioctl(struct tty_struct *tty, struct file *file,
--
1.6.6
On Thu, Dec 31, 2009 at 12:40:25PM +0100, Johan Hovold wrote:
> Please disregard this one as I just realised that this has already been
> fixed by Pete Zaitcev (in Greg's patch tree). The patches basically only
> differs in that I added error messages for failed allocations, and
> that Pete used put_unaligned for the divsor.
>
> Sorry about that.
No problem, thanks for letting me know.
greg k-h
On Thu, Dec 31, 2009 at 04:47:56PM +0100, Johan Hovold wrote:
> Hi,
>
> Here's an updated series rebased against rc2 with all patches from Greg's tree
> applied. The mct_u232 patch has been dropped as it had already been fixed. I
> also updated the ch341 and cypress_m8 patches to use the unaligned macros.
Ok, I took your original series (sorry, didn't read ahead) and then
added 2 new ones for the ch341 and cypress_m8 unaligned stuff. Please
verify that I got all of this correct.
thanks,
greg k-h
On Fri, Jan 15, 2010 at 10:50:34AM -0800, Greg KH wrote:
> On Thu, Dec 31, 2009 at 04:47:56PM +0100, Johan Hovold wrote:
> > Hi,
> >
> > Here's an updated series rebased against rc2 with all patches from Greg's tree
> > applied. The mct_u232 patch has been dropped as it had already been fixed. I
> > also updated the ch341 and cypress_m8 patches to use the unaligned macros.
>
> Ok, I took your original series (sorry, didn't read ahead) and then
> added 2 new ones for the ch341 and cypress_m8 unaligned stuff. Please
> verify that I got all of this correct.
Looks good.
/Johan