| Message ID | 20240504092504.24158-2-savicaleksa83@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | hwmon: (corsair-cpro) Fix issues when hidraw is used | expand |
On 04.05.24 at 11:25:01 MESZ, Aleksa Savic wrote > Introduce cmd_buffer, a separate buffer for storing only > the command that is sent to the device. Before this separation, > the existing buffer was shared for both the command and the > report received in ccp_raw_event(), which was copied into it. > > However, because of hidraw, the raw event parsing may be triggered > in the middle of sending a command, resulting in outputting gibberish > to the device. Using a separate buffer resolves this. > > Fixes: 40c3a4454225 ("hwmon: add Corsair Commander Pro driver") > Signed-off-by: Aleksa Savic <savicaleksa83@gmail.com> > --- > drivers/hwmon/corsair-cpro.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/drivers/hwmon/corsair-cpro.c b/drivers/hwmon/corsair-cpro.c > index a284a02839fb..8d85f66f8143 100644 > --- a/drivers/hwmon/corsair-cpro.c > +++ b/drivers/hwmon/corsair-cpro.c > @@ -79,6 +79,7 @@ struct ccp_device { > struct device *hwmon_dev; > struct completion wait_input_report; > struct mutex mutex; /* whenever buffer is used, lock before send_usb_cmd */ > + u8 *cmd_buffer; > u8 *buffer; > int target[6]; > DECLARE_BITMAP(temp_cnct, NUM_TEMP_SENSORS); > @@ -111,15 +112,15 @@ static int send_usb_cmd(struct ccp_device *ccp, u8 command, u8 byte1, u8 byte2, > unsigned long t; > int ret; > > - memset(ccp->buffer, 0x00, OUT_BUFFER_SIZE); > - ccp->buffer[0] = command; > - ccp->buffer[1] = byte1; > - ccp->buffer[2] = byte2; > - ccp->buffer[3] = byte3; > + memset(ccp->cmd_buffer, 0x00, OUT_BUFFER_SIZE); > + ccp->cmd_buffer[0] = command; > + ccp->cmd_buffer[1] = byte1; > + ccp->cmd_buffer[2] = byte2; > + ccp->cmd_buffer[3] = byte3; > > reinit_completion(&ccp->wait_input_report); > > - ret = hid_hw_output_report(ccp->hdev, ccp->buffer, OUT_BUFFER_SIZE); > + ret = hid_hw_output_report(ccp->hdev, ccp->cmd_buffer, OUT_BUFFER_SIZE); > if (ret < 0) > return ret; > > @@ -492,7 +493,11 @@ static int ccp_probe(struct hid_device *hdev, const struct hid_device_id *id) > if (!ccp) > return -ENOMEM; > > - ccp->buffer = devm_kmalloc(&hdev->dev, OUT_BUFFER_SIZE, GFP_KERNEL); > + ccp->cmd_buffer = devm_kmalloc(&hdev->dev, OUT_BUFFER_SIZE, GFP_KERNEL); > + if (!ccp->cmd_buffer) > + return -ENOMEM; > + > + ccp->buffer = devm_kmalloc(&hdev->dev, IN_BUFFER_SIZE, GFP_KERNEL); > if (!ccp->buffer) > return -ENOMEM; > > Thank you! Acked-by: Marius Zachmann <mail@mariuszachmann.de>
diff --git a/drivers/hwmon/corsair-cpro.c b/drivers/hwmon/corsair-cpro.c index a284a02839fb..8d85f66f8143 100644 --- a/drivers/hwmon/corsair-cpro.c +++ b/drivers/hwmon/corsair-cpro.c @@ -79,6 +79,7 @@ struct ccp_device { struct device *hwmon_dev; struct completion wait_input_report; struct mutex mutex; /* whenever buffer is used, lock before send_usb_cmd */ + u8 *cmd_buffer; u8 *buffer; int target[6]; DECLARE_BITMAP(temp_cnct, NUM_TEMP_SENSORS); @@ -111,15 +112,15 @@ static int send_usb_cmd(struct ccp_device *ccp, u8 command, u8 byte1, u8 byte2, unsigned long t; int ret; - memset(ccp->buffer, 0x00, OUT_BUFFER_SIZE); - ccp->buffer[0] = command; - ccp->buffer[1] = byte1; - ccp->buffer[2] = byte2; - ccp->buffer[3] = byte3; + memset(ccp->cmd_buffer, 0x00, OUT_BUFFER_SIZE); + ccp->cmd_buffer[0] = command; + ccp->cmd_buffer[1] = byte1; + ccp->cmd_buffer[2] = byte2; + ccp->cmd_buffer[3] = byte3; reinit_completion(&ccp->wait_input_report); - ret = hid_hw_output_report(ccp->hdev, ccp->buffer, OUT_BUFFER_SIZE); + ret = hid_hw_output_report(ccp->hdev, ccp->cmd_buffer, OUT_BUFFER_SIZE); if (ret < 0) return ret; @@ -492,7 +493,11 @@ static int ccp_probe(struct hid_device *hdev, const struct hid_device_id *id) if (!ccp) return -ENOMEM; - ccp->buffer = devm_kmalloc(&hdev->dev, OUT_BUFFER_SIZE, GFP_KERNEL); + ccp->cmd_buffer = devm_kmalloc(&hdev->dev, OUT_BUFFER_SIZE, GFP_KERNEL); + if (!ccp->cmd_buffer) + return -ENOMEM; + + ccp->buffer = devm_kmalloc(&hdev->dev, IN_BUFFER_SIZE, GFP_KERNEL); if (!ccp->buffer) return -ENOMEM;
Introduce cmd_buffer, a separate buffer for storing only the command that is sent to the device. Before this separation, the existing buffer was shared for both the command and the report received in ccp_raw_event(), which was copied into it. However, because of hidraw, the raw event parsing may be triggered in the middle of sending a command, resulting in outputting gibberish to the device. Using a separate buffer resolves this. Fixes: 40c3a4454225 ("hwmon: add Corsair Commander Pro driver") Signed-off-by: Aleksa Savic <savicaleksa83@gmail.com> --- drivers/hwmon/corsair-cpro.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)