From patchwork Fri Jan 12 14:35:06 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Carpenter X-Patchwork-Id: 762573 Received: from mail-wm1-f50.google.com (mail-wm1-f50.google.com [209.85.128.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 36D2F64CFF for ; Fri, 12 Jan 2024 14:35:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="nvzGNpBb" Received: by mail-wm1-f50.google.com with SMTP id 5b1f17b1804b1-40e63bc90f2so9382435e9.2 for ; Fri, 12 Jan 2024 06:35:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1705070110; x=1705674910; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:message-id:subject:cc :to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=Y95Jfas4yPMpMpOy2M2wHIojPX+ZKuNke1Ebmh6JQsU=; b=nvzGNpBb/QggawV0OZUTjf9VYCxKeVJAa+RE7Fls9OnS6P+uI1mV/o3Pr7kaSmlfhd cEksUFnc5LSafqYTs1zCj8+TaC12GxoqVxNCRg/em+0URqlvaScO9UbjUQJZUSm52xZN 7e4A6Fuu8H98dbv+G8NmdI6psx1qHl1ObdSOP9c0k/3scSsajY1Yo3y5w33Q8/nJBmYK 2q+8Fd+YLkJHQn1fFE69kjZo80xW8sHs1VIGhi1/s4o1c421w1TkBh/sZx7IqiqdUFRf yAhXHz02KVI6deJbztdmvP3lgqcek/arBdZ/5vs8LjhlzQ4t90GmFI4SaS0OGb+mnKaT caCg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705070110; x=1705674910; h=in-reply-to:content-disposition:mime-version:message-id:subject:cc :to:from:date:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Y95Jfas4yPMpMpOy2M2wHIojPX+ZKuNke1Ebmh6JQsU=; b=FaHCS7CafDNhn/KHWuVi1tL6YTRldwvvzXhGbdFeaJkLhuyjv8FCeym9JTyfKmvUTj LGUP0S2kTVWr6d7pENcqSw+HmcnXaPYma/g14Nk0mi1E/k7Ztf1vcyVU9ku+xaVSppva P+unj8C5VZoQc0lZrPq3OOsVx5zCWMFVdnVEPqylDyqe3icifoGFRSBYyd9Ke/LIGS6U tYqmmlWISI+BGs1mQtA7FbZvmpxj6IGmdogk1jUhMyb5en3W8W1R0IbUvvcey8Vldxkh 8plU8x8FvQUrm/sPOUCR5Ynxcwp1aMyvHUn3F3GlBnSIjVK2QLnW3RdelChusMLPtKIN vmkg== X-Gm-Message-State: AOJu0YxKMJVfgBigcVzmJCuxTl9s8a5rX35VDfm6rdlMhwISzoAvJGMh z3D5yexNlzX04+KRkbM58z1VyPhPj8wNWQ== X-Google-Smtp-Source: AGHT+IF5Ymenj6Zi2wDPf9weDy1XyZvZJoiZKVlmUQZsJUDWY62gba1DbI8h7VfF0Hw5vd0BEhf7ZQ== X-Received: by 2002:a1c:7c08:0:b0:40e:55bb:a0b8 with SMTP id x8-20020a1c7c08000000b0040e55bba0b8mr787274wmc.157.1705070110611; Fri, 12 Jan 2024 06:35:10 -0800 (PST) Received: from localhost ([102.140.209.237]) by smtp.gmail.com with ESMTPSA id j28-20020a05600c1c1c00b0040e6726befcsm2090812wms.10.2024.01.12.06.35.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Jan 2024 06:35:10 -0800 (PST) Date: Fri, 12 Jan 2024 17:35:06 +0300 From: Dan Carpenter To: Vicki Pfau Cc: Jiri Kosina , Benjamin Tissoires , linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: [PATCH 2/2] HID: hid-steam: Fix cleanup in probe() Message-ID: <1fd87904-dabf-4879-bb89-72d13ebfc91e@moroto.mountain> Precedence: bulk X-Mailing-List: linux-input@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <305898fb-6bd4-4749-806c-05ec51bbeb80@moroto.mountain> X-Mailer: git-send-email haha only kidding There are a number of issues in this code. First of all if steam_create_client_hid() fails then it leads to an error pointer dereference when we call hid_destroy_device(steam->client_hdev). Also there are a number of leaks. hid_hw_stop() is not called if hid_hw_open() fails for example. And it doesn't call steam_unregister() or hid_hw_close(). Fixes: 691ead124a0c ("HID: hid-steam: Clean up locking") Signed-off-by: Dan Carpenter --- This is just from static analysis and code review. I haven't tested it. I only included the fixes tag for the error pointer dereference. drivers/hid/hid-steam.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c index 59df6ead7b54..b08a5ab58528 100644 --- a/drivers/hid/hid-steam.c +++ b/drivers/hid/hid-steam.c @@ -1128,14 +1128,14 @@ static int steam_probe(struct hid_device *hdev, */ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDRAW); if (ret) - goto hid_hw_start_fail; + goto err_cancel_work; ret = hid_hw_open(hdev); if (ret) { hid_err(hdev, "%s:hid_hw_open\n", __func__); - goto hid_hw_open_fail; + goto err_hw_stop; } if (steam->quirks & STEAM_QUIRK_WIRELESS) { @@ -1151,33 +1151,37 @@ static int steam_probe(struct hid_device *hdev, hid_err(hdev, "%s:steam_register failed with error %d\n", __func__, ret); - goto input_register_fail; + goto err_hw_close; } } steam->client_hdev = steam_create_client_hid(hdev); if (IS_ERR(steam->client_hdev)) { ret = PTR_ERR(steam->client_hdev); - goto client_hdev_fail; + goto err_stream_unregister; } steam->client_hdev->driver_data = steam; ret = hid_add_device(steam->client_hdev); if (ret) - goto client_hdev_add_fail; + goto err_destroy; return 0; -client_hdev_add_fail: - hid_hw_stop(hdev); -client_hdev_fail: +err_destroy: hid_destroy_device(steam->client_hdev); -input_register_fail: -hid_hw_open_fail: -hid_hw_start_fail: +err_stream_unregister: + if (steam->connected) + steam_unregister(steam); +err_hw_close: + hid_hw_close(hdev); +err_hw_stop: + hid_hw_stop(hdev); +err_cancel_work: cancel_work_sync(&steam->work_connect); cancel_delayed_work_sync(&steam->mode_switch); cancel_work_sync(&steam->rumble_work); + return ret; }