All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Wu <peter@lekensteyn.nl>
To: Benjamin Tissoires <benjamin.tissoires@redhat.com>,
	Jiri Kosina <jkosina@suse.cz>,
	Nestor Lopez Casado <nlopezcasad@logitech.com>,
	Andrew de los Reyes <adlr@chromium.org>
Cc: Peter Hutterer <peter.hutterer@who-t.net>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 4/4] HID: logitech-hidpp: disable io in probe error path
Date: Thu, 11 Dec 2014 13:51:20 +0100	[thread overview]
Message-ID: <1418302280-14794-5-git-send-email-peter@lekensteyn.nl> (raw)
In-Reply-To: <1418302280-14794-1-git-send-email-peter@lekensteyn.nl>

Balance a hid_device_io_start() call with hid_device_io_stop() in the
error path. This avoids processing of HID reports when the probe fails
which possibly leads to invalid memory access in hid_device_probe() as
report_enum->report_id_hash might already be freed via
hid_close_report().

hid_set_drvdata() is called before wtp_allocate, be consistent and clear
drvdata too on the error path of wtp_allocate.

Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
Hi Andrew,

There are few users of hid_device_io_start/stop, this is the first one
to get start/stop out of sync. Should the comment of
hid_device_io_start() be updated to ensure that hid_device_io_stop()
gets called before probe() returns? Or should the hid-core be changed to
handle this out-of-sync issue:

		if (ret) {
			if (hdev->io_started))
				down(&hdev->driver_input_lock);
			hid_close_report(hdev);
			hdev->driver = NULL;
		}

Is my observation correct or not that HID reports can arrive during
hid_close_report() when io_started == true?

Kind regards,
Peter
---
 drivers/hid/hid-logitech-hidpp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 4292cc3..2f420c0 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -1121,7 +1121,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
 		ret = wtp_allocate(hdev, id);
 		if (ret)
-			return ret;
+			goto wtp_allocate_fail;
 	}
 
 	INIT_WORK(&hidpp->work, delayed_work_cb);
@@ -1141,6 +1141,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (id->group != HID_GROUP_LOGITECH_DJ_DEVICE) {
 		if (!connected) {
 			hid_err(hdev, "Device not connected");
+			hid_device_io_stop(hdev);
 			goto hid_parse_fail;
 		}
 
@@ -1186,6 +1187,7 @@ hid_hw_start_fail:
 hid_parse_fail:
 	cancel_work_sync(&hidpp->work);
 	mutex_destroy(&hidpp->send_mutex);
+wtp_allocate_fail:
 	hid_set_drvdata(hdev, NULL);
 	return ret;
 }
-- 
2.1.3


  parent reply	other threads:[~2014-12-11 12:52 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-11 12:51 [PATCH 0/4] HID: logitech-hidpp: fixes for error conditions Peter Wu
2014-12-11 12:51 ` [PATCH 1/4] HID: logitech-hidpp: do not return the name length Peter Wu
2014-12-11 15:22   ` Benjamin Tissoires
2014-12-11 12:51 ` [PATCH 2/4] HID: logitech-hidpp: check name retrieval return code Peter Wu
2014-12-11 15:23   ` Benjamin Tissoires
2014-12-11 12:51 ` [PATCH 3/4] HID: logitech-hidpp: add boundary check for name retrieval Peter Wu
2014-12-11 15:25   ` Benjamin Tissoires
2014-12-11 12:51 ` Peter Wu [this message]
2014-12-11 15:31   ` [PATCH 4/4] HID: logitech-hidpp: disable io in probe error path Benjamin Tissoires
2014-12-11 17:37     ` Andrew de los Reyes
2014-12-11 17:53       ` Peter Wu
2014-12-11 22:11         ` Andrew de los Reyes
2014-12-11 22:11 ` [PATCH 0/4] HID: logitech-hidpp: fixes for error conditions Jiri Kosina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1418302280-14794-5-git-send-email-peter@lekensteyn.nl \
    --to=peter@lekensteyn.nl \
    --cc=adlr@chromium.org \
    --cc=benjamin.tissoires@redhat.com \
    --cc=jkosina@suse.cz \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nlopezcasad@logitech.com \
    --cc=peter.hutterer@who-t.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.