From patchwork Fri Jan 12 14:34:14 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Carpenter X-Patchwork-Id: 13518549 X-Patchwork-Delegate: jikos@jikos.cz Received: from mail-wm1-f51.google.com (mail-wm1-f51.google.com [209.85.128.51]) (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 2CDBA6BB48 for ; Fri, 12 Jan 2024 14:34:18 +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="QSxNKtH0" Received: by mail-wm1-f51.google.com with SMTP id 5b1f17b1804b1-40e69b31365so351195e9.0 for ; Fri, 12 Jan 2024 06:34:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1705070057; x=1705674857; darn=vger.kernel.org; h=content-disposition:mime-version:message-id:subject:cc:to:from:date :from:to:cc:subject:date:message-id:reply-to; bh=wMAozsps9KhdnPC0gC/CqjxcAys0VIF1YUv+72UuGdM=; b=QSxNKtH0L5ny97b8ubTtPuslm7pNmNynFnHnzGI/mk4exHOX4oaK6f/LzjXq4m4Pgg S2PttR0ATVEnHHRuqAQ/hszkFViYWkW29xxS4hpqduKzbbE0KmkTN7Bv+XmNrZOz922v JFt9J4fTpVVMk7QEygpR3hHUoDTzQF6Hi1SzEXFWh5877WesQ1pZ6W54e92A+k+74k5F Jg+MDlwuDGu0zpZKLXxXtpmimmiwuPU4zs2BQxsAZTZbqcpSOk4PanE6D5B81vvrkooY vTPWxCu5EgZV07p640BUWol6qiBj4+Rdb2B8oEw4atT+ZZjkmhyKJIOC3rxPd729dIxO A0zA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705070057; x=1705674857; h=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=wMAozsps9KhdnPC0gC/CqjxcAys0VIF1YUv+72UuGdM=; b=FUwlo6Es0NOyipMTSeRPXFbYLsFjnTVulq+7/sIlDAfolnFmNy/Y3esnEmTs8itwVt MiJ/h7ibUuLAu3okAjsa8Z3eDPbG+eslRArlzfkzcFdAzLmvlyhwAGPB1ZmOOeL0fwwp xkfwfoRUvjoLgACoU4FXcXpRyRUfoLoqaoOb6gp5hjp8j9sQAtP9wY/eQiDmJhPOflpY 0SmZK/vXfQ1lcq3pjHDTutYiz0xAYemMAsywAMGSJEhrX22huLmr7eXDUk+gdfPB6Fxw TI67dWsFqIVQo8S82AWE7lKvXD2TCZr1CPZBLX30Eipp6tPM2FbIicJNvFHfM9/bs34+ tc8w== X-Gm-Message-State: AOJu0YzHytnooJAPsuIf0jlOtWDvZRoFgH5OhsPeaTPfVxB/jRPuqm6S foSxhuWtDtqBHKEAsJFiYGCBMr/Kt1/3AQ== X-Google-Smtp-Source: AGHT+IEo2mOleVaYUfc9j4l2dY3+TK3k54YTZMybDNjOmwmfLYfAMTaWAKbDYx/jgnOTy0AsBo3okg== X-Received: by 2002:a05:600c:1705:b0:40e:4832:9fab with SMTP id c5-20020a05600c170500b0040e48329fabmr886892wmn.143.1705070057428; Fri, 12 Jan 2024 06:34:17 -0800 (PST) Received: from localhost ([102.140.209.237]) by smtp.gmail.com with ESMTPSA id l17-20020a05600c1d1100b0040e4a2b36bfsm9849022wms.22.2024.01.12.06.34.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Jan 2024 06:34:17 -0800 (PST) Date: Fri, 12 Jan 2024 17:34:14 +0300 From: Dan Carpenter To: Jiri Kosina Cc: Benjamin Tissoires , linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: [PATCH 1/2] HID: hid-steam: remove pointless error message Message-ID: <305898fb-6bd4-4749-806c-05ec51bbeb80@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 X-Mailer: git-send-email haha only kidding This error message doesn't really add any information. If modprobe fails then the user will already know what the error code is. In the case of kmalloc() it's a style violation to print an error message for that because kmalloc has it's own better error messages built in. Signed-off-by: Dan Carpenter --- drivers/hid/hid-steam.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c index b3c4e50e248a..59df6ead7b54 100644 --- a/drivers/hid/hid-steam.c +++ b/drivers/hid/hid-steam.c @@ -1109,10 +1109,9 @@ static int steam_probe(struct hid_device *hdev, return hid_hw_start(hdev, HID_CONNECT_DEFAULT); steam = devm_kzalloc(&hdev->dev, sizeof(*steam), GFP_KERNEL); - if (!steam) { - ret = -ENOMEM; - goto steam_alloc_fail; - } + if (!steam) + return -ENOMEM; + steam->hdev = hdev; hid_set_drvdata(hdev, steam); spin_lock_init(&steam->lock); @@ -1179,9 +1178,6 @@ static int steam_probe(struct hid_device *hdev, cancel_work_sync(&steam->work_connect); cancel_delayed_work_sync(&steam->mode_switch); cancel_work_sync(&steam->rumble_work); -steam_alloc_fail: - hid_err(hdev, "%s: failed with error %d\n", - __func__, ret); return ret; } 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: 13518550 X-Patchwork-Delegate: jikos@jikos.cz 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 Reviewed-by: Vicki Pfau --- 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; }