| Message ID | 20180414150645.14876-2-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Apr 14, 2018 at 5:06 PM, Hans de Goede <hdegoede@redhat.com> wrote: > hid_ishtp_get_report() may be called by multiple callers at the same > time, causing trouble with the static local buffer used. > > Also there is no reason to use a non stack buffer, the buffer is tiny > and ishtp_cl_send() copies its contents so the lifetime is not an > issue either. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- Series looks good: Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Cheers, Benjamin > drivers/hid/intel-ish-hid/ishtp-hid-client.c | 16 ++++++---------- > 1 file changed, 6 insertions(+), 10 deletions(-) > > diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c > index 6ce1856bb368..acc2536c8094 100644 > --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c > +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c > @@ -412,9 +412,7 @@ void hid_ishtp_get_report(struct hid_device *hid, int report_id, > { > struct ishtp_hid_data *hid_data = hid->driver_data; > struct ishtp_cl_data *client_data = hid_data->client_data; > - static unsigned char buf[10]; > - unsigned int len; > - struct hostif_msg_to_sensor *msg = (struct hostif_msg_to_sensor *)buf; > + struct hostif_msg_to_sensor msg = {}; > int rv; > int i; > > @@ -426,14 +424,11 @@ void hid_ishtp_get_report(struct hid_device *hid, int report_id, > return; > } > > - len = sizeof(struct hostif_msg_to_sensor); > - > - memset(msg, 0, sizeof(struct hostif_msg_to_sensor)); > - msg->hdr.command = (report_type == HID_FEATURE_REPORT) ? > + msg.hdr.command = (report_type == HID_FEATURE_REPORT) ? > HOSTIF_GET_FEATURE_REPORT : HOSTIF_GET_INPUT_REPORT; > for (i = 0; i < client_data->num_hid_devices; ++i) { > if (hid == client_data->hid_sensor_hubs[i]) { > - msg->hdr.device_id = > + msg.hdr.device_id = > client_data->hid_devices[i].dev_id; > break; > } > @@ -442,8 +437,9 @@ void hid_ishtp_get_report(struct hid_device *hid, int report_id, > if (i == client_data->num_hid_devices) > return; > > - msg->report_id = report_id; > - rv = ishtp_cl_send(client_data->hid_ishtp_cl, buf, len); > + msg.report_id = report_id; > + rv = ishtp_cl_send(client_data->hid_ishtp_cl, (uint8_t *)&msg, > + sizeof(msg)); > if (rv) > hid_ishtp_trace(client_data, "%s hid %p send failed\n", > __func__, hid); > -- > 2.17.0 > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 2018-04-14 at 17:06 +0200, Hans de Goede wrote: > hid_ishtp_get_report() may be called by multiple callers at the same > time, causing trouble with the static local buffer used. > > Also there is no reason to use a non stack buffer, the buffer is tiny > and ishtp_cl_send() copies its contents so the lifetime is not an > issue either. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > --- > drivers/hid/intel-ish-hid/ishtp-hid-client.c | 16 ++++++---------- > 1 file changed, 6 insertions(+), 10 deletions(-) > > diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c > b/drivers/hid/intel-ish-hid/ishtp-hid-client.c > index 6ce1856bb368..acc2536c8094 100644 > --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c > +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c > @@ -412,9 +412,7 @@ void hid_ishtp_get_report(struct hid_device *hid, > int report_id, > { > struct ishtp_hid_data *hid_data = hid->driver_data; > struct ishtp_cl_data *client_data = hid_data->client_data; > - static unsigned char buf[10]; > - unsigned int len; > - struct hostif_msg_to_sensor *msg = (struct > hostif_msg_to_sensor *)buf; > + struct hostif_msg_to_sensor msg = {}; > int rv; > int i; > > @@ -426,14 +424,11 @@ void hid_ishtp_get_report(struct hid_device > *hid, int report_id, > return; > } > > - len = sizeof(struct hostif_msg_to_sensor); > - > - memset(msg, 0, sizeof(struct hostif_msg_to_sensor)); > - msg->hdr.command = (report_type == HID_FEATURE_REPORT) ? > + msg.hdr.command = (report_type == HID_FEATURE_REPORT) ? > HOSTIF_GET_FEATURE_REPORT : HOSTIF_GET_INPUT_REPORT; > for (i = 0; i < client_data->num_hid_devices; ++i) { > if (hid == client_data->hid_sensor_hubs[i]) { > - msg->hdr.device_id = > + msg.hdr.device_id = > client_data->hid_devices[i].dev_id; > break; > } > @@ -442,8 +437,9 @@ void hid_ishtp_get_report(struct hid_device *hid, > int report_id, > if (i == client_data->num_hid_devices) > return; > > - msg->report_id = report_id; > - rv = ishtp_cl_send(client_data->hid_ishtp_cl, buf, len); > + msg.report_id = report_id; > + rv = ishtp_cl_send(client_data->hid_ishtp_cl, (uint8_t > *)&msg, > + sizeof(msg)); > if (rv) > hid_ishtp_trace(client_data, "%s hid %p send > failed\n", > __func__, hid); -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c index 6ce1856bb368..acc2536c8094 100644 --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c @@ -412,9 +412,7 @@ void hid_ishtp_get_report(struct hid_device *hid, int report_id, { struct ishtp_hid_data *hid_data = hid->driver_data; struct ishtp_cl_data *client_data = hid_data->client_data; - static unsigned char buf[10]; - unsigned int len; - struct hostif_msg_to_sensor *msg = (struct hostif_msg_to_sensor *)buf; + struct hostif_msg_to_sensor msg = {}; int rv; int i; @@ -426,14 +424,11 @@ void hid_ishtp_get_report(struct hid_device *hid, int report_id, return; } - len = sizeof(struct hostif_msg_to_sensor); - - memset(msg, 0, sizeof(struct hostif_msg_to_sensor)); - msg->hdr.command = (report_type == HID_FEATURE_REPORT) ? + msg.hdr.command = (report_type == HID_FEATURE_REPORT) ? HOSTIF_GET_FEATURE_REPORT : HOSTIF_GET_INPUT_REPORT; for (i = 0; i < client_data->num_hid_devices; ++i) { if (hid == client_data->hid_sensor_hubs[i]) { - msg->hdr.device_id = + msg.hdr.device_id = client_data->hid_devices[i].dev_id; break; } @@ -442,8 +437,9 @@ void hid_ishtp_get_report(struct hid_device *hid, int report_id, if (i == client_data->num_hid_devices) return; - msg->report_id = report_id; - rv = ishtp_cl_send(client_data->hid_ishtp_cl, buf, len); + msg.report_id = report_id; + rv = ishtp_cl_send(client_data->hid_ishtp_cl, (uint8_t *)&msg, + sizeof(msg)); if (rv) hid_ishtp_trace(client_data, "%s hid %p send failed\n", __func__, hid);
hid_ishtp_get_report() may be called by multiple callers at the same time, causing trouble with the static local buffer used. Also there is no reason to use a non stack buffer, the buffer is tiny and ishtp_cl_send() copies its contents so the lifetime is not an issue either. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/hid/intel-ish-hid/ishtp-hid-client.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)