| Message ID | 20200716085056.31471-1-johan@kernel.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | de37458f8c2bfc465500a1dd0d15dbe96d2a698c |
| Headers | show |
| Series | USB: serial: iuu_phoenix: fix led-activity helpers | expand |
On Thu, Jul 16, 2020 at 10:50:55AM +0200, Johan Hovold wrote: > The set-led command is eight bytes long and starts with a command byte > followed by six bytes of RGB data and ends with a byte encoding a > frequency (see iuu_led() and iuu_rgbf_fill_buffer()). > > The led activity helpers had a few long-standing bugs which corrupted > the command packets by inserting a second command byte and thereby > offsetting the RGB data and dropping the frequency in non-xmas mode. > > In xmas mode, a related off-by-one error left the frequency field > uninitialised. > > Fixes: 60a8fc017103 ("USB: add iuu_phoenix driver") > Reported-by: George Spelvin <lkml@sdf.org> > Signed-off-by: Johan Hovold <johan@kernel.org> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Thu, Jul 16, 2020 at 11:05:03AM +0200, Greg Kroah-Hartman wrote: > On Thu, Jul 16, 2020 at 10:50:55AM +0200, Johan Hovold wrote: > > The set-led command is eight bytes long and starts with a command byte > > followed by six bytes of RGB data and ends with a byte encoding a > > frequency (see iuu_led() and iuu_rgbf_fill_buffer()). > > > > The led activity helpers had a few long-standing bugs which corrupted > > the command packets by inserting a second command byte and thereby > > offsetting the RGB data and dropping the frequency in non-xmas mode. > > > > In xmas mode, a related off-by-one error left the frequency field > > uninitialised. > > > > Fixes: 60a8fc017103 ("USB: add iuu_phoenix driver") > > Reported-by: George Spelvin <lkml@sdf.org> > > Signed-off-by: Johan Hovold <johan@kernel.org> > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Thanks for reviewing. Now applied for -next. Johan
diff --git a/drivers/usb/serial/iuu_phoenix.c b/drivers/usb/serial/iuu_phoenix.c index 0c2c4aea289c..b4ba79123d9d 100644 --- a/drivers/usb/serial/iuu_phoenix.c +++ b/drivers/usb/serial/iuu_phoenix.c @@ -350,10 +350,11 @@ static void iuu_led_activity_on(struct urb *urb) { struct usb_serial_port *port = urb->context; char *buf_ptr = port->write_urb->transfer_buffer; - *buf_ptr++ = IUU_SET_LED; + if (xmas) { - get_random_bytes(buf_ptr, 6); - *(buf_ptr+7) = 1; + buf_ptr[0] = IUU_SET_LED; + get_random_bytes(buf_ptr + 1, 6); + buf_ptr[7] = 1; } else { iuu_rgbf_fill_buffer(buf_ptr, 255, 255, 0, 0, 0, 0, 255); } @@ -370,13 +371,14 @@ static void iuu_led_activity_off(struct urb *urb) { struct usb_serial_port *port = urb->context; char *buf_ptr = port->write_urb->transfer_buffer; + if (xmas) { iuu_rxcmd(urb); return; - } else { - *buf_ptr++ = IUU_SET_LED; - iuu_rgbf_fill_buffer(buf_ptr, 0, 0, 255, 255, 0, 0, 255); } + + iuu_rgbf_fill_buffer(buf_ptr, 0, 0, 255, 255, 0, 0, 255); + usb_fill_bulk_urb(port->write_urb, port->serial->dev, usb_sndbulkpipe(port->serial->dev, port->bulk_out_endpointAddress),
The set-led command is eight bytes long and starts with a command byte followed by six bytes of RGB data and ends with a byte encoding a frequency (see iuu_led() and iuu_rgbf_fill_buffer()). The led activity helpers had a few long-standing bugs which corrupted the command packets by inserting a second command byte and thereby offsetting the RGB data and dropping the frequency in non-xmas mode. In xmas mode, a related off-by-one error left the frequency field uninitialised. Fixes: 60a8fc017103 ("USB: add iuu_phoenix driver") Reported-by: George Spelvin <lkml@sdf.org> Signed-off-by: Johan Hovold <johan@kernel.org> --- George, Thanks for reporting this issue and sorry for not getting back to you sooner about this. It was sort buried in that long RFC series of yours which was about something unrelated (and your patch did two distinct logical changes in one patch which is generally frowned upon): https://lore.kernel.org/r/202003281643.02SGhKg2024958@sdf.org Let me know if you prefer to respin your fix, and then include also the fix of the related issue in iuu_led_activity_off(). Otherwise I'll apply this one next week. Johan drivers/usb/serial/iuu_phoenix.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)