From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oo1-f52.google.com (mail-oo1-f52.google.com [209.85.161.52]) (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 896163CF695 for ; Fri, 10 Jul 2026 07:42:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.161.52 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783669360; cv=none; b=qk3MX6Qo4wNHCXH1+aa+UKx3GB2ikDEajy7fIsvk3jfL5abrmfgUf5VcGJ3Exn/pCwmKdrBJ032kKwU5fzYEdHwiaYtFH4IN+Ge95TxFsRDF/xjYFLalKh+OJhvZmDKDPmy96bb/W166CL1aAnOS78c7kzqLtAYKV6WW3950am4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783669360; c=relaxed/simple; bh=fJyw3EwJS+C7+OwdfFKFVk+jzCS6vs1g2XNuSa2azqE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=VFpnDAXQk0zz+U9lN9EcBu0ERMxW9PkmHxGfgGj2t6hfjX/z//AZKPEZQ4sSSkMdFZjkLUnElIQCrghpa9kWGkEhqg0tq6QKnWFEwaKnncfQ6lrgqN/mzgf6a47GgEN7g2jK+BCdqI0t0ugxYuDPGh4NkhkVVqKK2sSLihoN3qo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=jHnAcGbE; arc=none smtp.client-ip=209.85.161.52 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="jHnAcGbE" Received: by mail-oo1-f52.google.com with SMTP id 006d021491bc7-6a379b68f76so262212eaf.3 for ; Fri, 10 Jul 2026 00:42:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1783669358; x=1784274158; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=kNMWIhd4Kn6qK++so+3j3gVzZtTmvgc+EmxNrLVjwPQ=; b=jHnAcGbEfVamzQi/BIQr2+5S/TL/s/Rqm2088D1CsYNJu+7gmY56UwQU+oHLZKaf/P h4UHS0668yemtEcVV3LW10yrr6pEHuUBqWWRjoiH/gu33vZfehyKcUajnoZvf+QxAebo m6/hEvim5RKfdSJOlHOSji9uK25t/y8W6d6EXBiYpjE4zlQiy/kDQHVyNeZcZsAn5bd2 BSYiKclk7efOwOAeOawTi84eLOS4oNEQ0YUq8EcnZ/KVXJtPNMIFnQU3LROelYLQKzKn pnHlxsC+sgp4RsHSeAgvAAK3n3BJekopUFAdr2b01k6dbWaac7X2QY5Dz07SKj3VP3B6 l/Hg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783669358; x=1784274158; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=kNMWIhd4Kn6qK++so+3j3gVzZtTmvgc+EmxNrLVjwPQ=; b=N4k4XZ9P3OVvrgetyt9j7X5rA3lqv9kbfkyoxWeFvZqbmGXOWUfB48pJ8ZUz+6WL5r nEUwPobqC6VhFTuxwRk+o1Eoi74mKHXW7b7KVNIcVBwuy9DMn8jqG3LBwWdrK9mzmRW3 wPUrtWsLmF3iBrmHDAkbtOssZKhF9BFQafr918ST/pPkpN+Xg/ErJgQdTepmyobw268b seJXXRv8/lJksCr/gcpVokJlLpVA95bmT/DYing0JTlHl+9G1L9KlqdDP4NjN3Qxx0lQ tFMiAiiHwsysEmV14PdgxlTvUkvBFEqopqjEOpfqTdRVnKcFmlmsJ8FC43iMW7SSa7Bk l0ew== X-Forwarded-Encrypted: i=1; AFNElJ+8vieKdKh1SwzSBKy+L2vTdhcz1+Z6Cae2DFCOwP7Vp3RifMH2keHT+DWrwSYCwUkvwyrAJkQzASUI9Zw=@vger.kernel.org X-Gm-Message-State: AOJu0Yydap6Qew8BhPNji3ShkZxN50FKfmh5vAyMTd8MHUwHa8c7GfZW FOwR2IfbQ6z04wacna9RwqGv98njrWld7U1lDYNwMCSxYx56OgNJHigQ9Ibo+13UXIdWFA== X-Gm-Gg: AfdE7ckJFNh3MfUBIW8JAJvzn5pi6JTpujTrjGLIS9o4ummdhzrvpV+Ny+4TpuutbQa gU3Ge9EnFKnk1HGsg6FKOumNVonwSTqjv85Nh9de7Xp7BA4VLR8wDyg3t8Cz2+VI0Hwiwmbx7Rl AMY+WEKI+dQD+N3JtjwzOdw5QLOv8Hdi4kl/1Sbuxi725A8h9kU0mT4uafawZzKLV+FWlo2/NFU yPmSu+dMGurrhEd2kxzFvsPuhVxvWmsA5qd7iIFj22tEr5fo7p96z2bPBD2QJBGWzu2gfuft50u w5DhXp3AbfRGm9R7YiVQKdpKBsxJfHrva2dEzciDinJfgVQcpq6/qW3afKc1BfvVd1sOW3MF3gt T2RpPWdnJyZSWidMCamORDM7uYbJUQAJ1w5ntuuRMfjHdec0FdcdabGv6PmN9KsuBmHJfYPuB+W NQfjTw X-Received: by 2002:a05:6820:981:b0:6a3:8023:f343 with SMTP id 006d021491bc7-6a38023f3b7mr3977072eaf.16.1783669358539; Fri, 10 Jul 2026 00:42:38 -0700 (PDT) Received: from localhost ([74.80.182.70]) by smtp.gmail.com with ESMTPSA id 006d021491bc7-6a38b9ccff2sm1158317eaf.3.2026.07.10.00.42.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Jul 2026 00:42:36 -0700 (PDT) Date: Fri, 10 Jul 2026 10:42:31 +0300 From: Dan Carpenter To: Hao-Qun Huang Cc: Greg Kroah-Hartman , Viresh Kumar , Johan Hovold , Alex Elder , greybus-dev@lists.linaro.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging: greybus: hid: fix SET_REPORT return value Message-ID: References: <20260704081613.434445-1-alvinhuang0603@gmail.com> <20260710020644.6728.alvinhuang0603@gmail.com> <20260710124050.31335.alvinhuang0603@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260710124050.31335.alvinhuang0603@gmail.com> So the patch has already been merged and it's fine. The fix is correct. On Fri, Jul 10, 2026 at 12:40:50PM +0800, Hao-Qun Huang wrote: > On Jul 10, 2026 at 2:50 AM, Dan Carpenter wrote: > > The bug is not hard to understand, the issue is that this changes the > > function completely... Was nothing checking the return before? > > It gets checked. The value flows unchanged through __hid_hw_raw_request() > to the callers, and they look at it two ways: > > - hidraw returns it straight to userspace (write(), HIDIOCSFEATURE), > where it is the number of bytes transferred. > > - in-kernel SET_REPORT callers, some testing "ret < 0" (hid-multitouch, > hid-sony), some testing "ret != size" (hid-gt683r, hid-lenovo, > hid-razer). > > So the old return 0 was wrong both ways: the first group had a failed > SET_REPORT masked as success, and the second saw every SET_REPORT as a > failure. Returning the count or a negative errno is what GET already > does in this driver and what usbhid/i2c-hid/uhid return, so nothing > working with those relied on the 0. What I'm trying to say is, sure, it's easy to see the code is buggy but it's been that way for years. Your patch changes the return completely from always returning zero to never returning zero. When we're reviewing this patch we want to know how making that change is safe. In staging often the answer is that nothing was calling that function and we can delete it... But here, the real answer is that almost nothing checks for errors. For the few places that do, almost all of them only check for negatives. That's probably how the code was able to work as it is... The commit message it should explain the risks. "Changing this code is fine because almost nothing checks for errors. There are a one or two in kernel checks which care about the exact positive return and this patch will fix that but basically not much is affected. And hopefully userspace doesn't check either or it only checks for negative errors. But in the spirit of correctness, lets change this return to be return the number of bytes and if userspace needs adjusting we will deal with that when users file their bug reports." regards, dan carpenter