From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from vern.gendns.com (vern.gendns.com [98.142.107.122]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4AC0B1BD4F7; Thu, 8 May 2025 15:21:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=98.142.107.122 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1746717701; cv=none; b=Ea1PhgvK0l4C1mVuLVYEG+5R8+i/KUyRLo7TMJvaSQ6MOT4Nacnd/DJ4lMASVeuxjLNHxcEGeQBjH00zlbQQ9v+Z1+AOBOyUNdjW4+TSwV0ZzSwBCHnlnh8oo7Fj6cuahJJBLkSASiV/cWhp2eiX1ueFcLqqo/q2f81ikiPP7+Q= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1746717701; c=relaxed/simple; bh=UK1pyBSP6cnOEUNhCJEzF4QCBjLicUgO5zbNc1oDH28=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=l2axoB1sfS7Mh/eKvPv6nwbMBkfk1LWAwHeEabY4tQWo1jSHmgu9fCmHb1nWatw1YWxmBf5mczNxVdNXdHAgkfRrCI8k2O1q5N/nNaw5WtxGeiShpJ7t2Bl/MsIdqAdA4fdugRPK0QbLMRCSo0amg22Z1ubVlQuv9HMdqJXnQQ8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=lechnology.com; spf=pass smtp.mailfrom=lechnology.com; dkim=pass (2048-bit key) header.d=lechnology.com header.i=@lechnology.com header.b=faLioyyP; arc=none smtp.client-ip=98.142.107.122 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=lechnology.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=lechnology.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=lechnology.com header.i=@lechnology.com header.b="faLioyyP" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lechnology.com; s=default; h=Content-Transfer-Encoding:Content-Type: In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender :Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help: List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=LA0jiwSe+AcbAnMB83EVPyLYZNhKqox30NsMzUL+r88=; b=faLioyyPDxhoxxZkhwpRhpq+CE r7crjGoLIywBtaGfSkiDc4FGJUGvfAsH3DxMBSP39qT+d6vgLuUwrRX1RvdD48TwsoLhnwlLMWQVh Z7pB0/yc+ZivFyN1E49ZpQfEE13xxKOqK5b+0IgCr1UtgCH0xwoYvGwA2A7VlcCwSXPU/PBS45fuP U3U5rVvYT8oTE9u7vnYFhdCSNh1RsE81Qq7eIUvoQ5h6ZVPUyikNX5r+Uc1P6lGuXxbT5sI5pacm5 mCy/UDnvWFO7ph5dlWE/7ZbevL9ecaIlTb0F2YNwlkVUQ2L3Hz4G5zbAVZw2onlAyryCpc//fprwI G5R1GqVg==; Received: from [2600:8803:e7e4:1d00:1120:d1cf:c64a:ac7e] (port=48630) by vern.gendns.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.1) (envelope-from ) id 1uD2p3-0000000024m-0AGg; Thu, 08 May 2025 11:03:12 -0400 Message-ID: <037fc605-3401-4e68-b452-b5e4882d56bc@lechnology.com> Date: Thu, 8 May 2025 10:03:09 -0500 Precedence: bulk X-Mailing-List: linux-leds@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] leds: uleds: fix unchecked copy_to_user() in uleds_read() To: Lee Jones , Ivan Stepchenko Cc: Pavel Machek , Jacek Anaszewski , linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org, lvc-project@linuxtesting.org References: <20250505081342.3855-1-sid@itb.spb.ru> <20250508143451.GQ3865826@google.com> From: David Lechner Content-Language: en-US Autocrypt: addr=david@lechnology.com; keydata= xsFNBFFxkZ8BEADXzbnj9t8XSZYxKJGHdHqYgEBVzRElb3+f11qhDZKzVCMsn1+AN+PlHqC7 VrCWLsWTSY7WsHB2fW3aXaoidtac5FYoX2IXAun1Sbv15NcBdapImkMv6zxhAyWz6LqPfdCp QV+3x6qwUPFeLHdmew8mkSq56qTFgDQr9oQhsrXKHkXFD7aIAf5bM6janQCHgGTVDraRDfEO rV9rj7Wu/SfjUCVSCvW/SuWBa3IXTLNgbrNwBfo7Pl/tHuto0jxkVCIJ6J3xa85BKMw1WjA+ jKzh12S6KWrLUfhEUt64G9WJHiZOnVAjxgCR7TUahVM2OQHcp49ouG/JZsGNniulXH4ErA2O Wt6seUEx8XQIm48H96RWgKrwKJ+1WoLEmUcYOJDZUcguMZVc3Astx8aSaRjf6IRBO8XlJSJV OorkguvrTQBZJfjoicuFx7VlpdMggMZayv0cqEvzZMSHUt8DCUG74rLhtab9LCg/9wdCwqyE JEi/8jaV7JWxwiCmzVpw0mHn1DiUlp5kapZT+Hart0Gc1WW915psA4G6KneisFM5DJe+S5mn dUJb5IttTOx37jQQi2igwlSBdSC/M+Zy3sb+DXYJUVjVxK56RGAnlSvjHUx/TkID6Vb6HXvm Fgm9vQamTEf+C3XzlY2v1YaMMX8yQjfrzQSoGfB0+9zaD9J/cwARAQABzSREYXZpZCBMZWNo bmVyIDxkYXZpZEBsZWNobm9sb2d5LmNvbT7CwdIEEwEIAIYFgmeVPmMECwkIBwkQH4r4jIL3 fANHFAAAAAAAHgAgc2FsdEBub3RhdGlvbnMuc2VxdW9pYS1wZ3Aub3JnDM6jI9LThow7adCF tC3vi3zrklAc6o/kt42Hifhjwk8DFQgKBBYCAwECF4ACGwMCHgEWIQSKc9gqah9QmQfzc4gf iviMgvd8AwAAEm4P/04Ou1k+zfSz2Di+wzFiIzz7c3zyU+R04sj0rFx4KRKIBYQQxgQOTkM/ zbKLMlggKMsbgICjDlWLp6ANCH0A22gGZQx5PJBDfjIl05G+GnK6XilpLyd3U18Xj/7PbB/t GHER2Llpf/ePe1YgZPqUuI7fTtFz5QLdIjr/ygb+HWJI/H/IydaJfFDWxQWU6quGi852oKv8 KMhmhGjgahPF+am6p0iPjkm+PfhHchxgKIneBixpwxFaOlikODcNuo0E+wp3gGLkaDIoGv15 H3BMZklu96EOKeKQYctpCj8RvTKzjEbn6JxGyXhVGoPMnic2Mwc0TNrXccqDqlQh48FEK6+L zAbQrPE3wWl1PFxSUvUc6b3jZ1JAjcVU2GfqhzHC0U1cjJX/XKA3jn60jl9vBgU+DkvT6Gq6 +pzj2nQszEx+N0+71I2v/vgoB8+kRKlibh2ydDRXfpipn2r4qR5imONrbW7OkLCEJ8nHmpmK N8iZKJjjTFmktLesE1s2L0hb9eoWz7i4YGCcIMOZISRTv/w860ebOrH787Bg3JNRz+edvKU8 TM3twZrCedbi+wBZcgGUBpPkWLH9dUTgpycjRcCOPqOzuHQIOqCMXWFq2cQ9Oy5szMdwsEzh Zf1Ys7e2++tAuALI/HXJNk4/BuddZYoorLyw7MV2mVEV91ERPIx4zsFNBFFxkZ8BEADSVjyc eG8Up24FFXwv5YmV7yX520kM97N11e1RJVMI1RSU+Na3Xo9J1BW6EFMAdibD6hH8PiMmToKx BrfYSLStLh2MbHA2T/3zqicU1nuk376LMyrAuoV/fl8/7Jldwh1c9AADaYXNQfZ84R6nyaTR jy4fqcc/dG2kw5ZMln909SMKZc3HdVynmo9pLT2HBOnXu2d3bIGmzuDnDXzh1X8+ods4gViu vB31xU1WiANr4TbhaNU+/LmEVfvhS+34Cmz3U5Xs5x7nWdpM6fFfDOSz2sIYXOGAcaV3oJ12 1Uul2U2bMTsXxiwdbjmZP9jrzEfvhD5KIOutX+0OzdtM9QVB70QQOEh3maW/FwGdL5stYcad sBiEEI6Y2ymVpBgzrPS6HzC+UZLUShOE+aLx+SYBYAuypikMPvG9W3MqWHCsXXEfyp2mCeor Kb7PafyaBO/E5REjPmYUpkGMNZH1lGV3jegE9WdOBfXW9xvCwf0UefoFaVhjsjtzvl8lMQnd rDBdKPpJ7zIIG6FGSsUYmCtvE+JAk83tfpUpSZKDSzsqtLTI8GE2fQzEuZcBqm6Yk2V1+u6r jUjmqEBIzunyeUupaUc+p00JiwNE8v/wcx7UbD5m+PGOkNoLMLe0ti0O7nFlY8avZzy3eLBQ enu4WsJjPVYeQGeGB3oLvCGIhT9/WwARAQABwsFfBBgBAgAJBQJRcZGfAhsMAAoJEB+K+IyC 93wDC44P/0bAjHgFUPHl7jG5CrWGwgdTNN8NrjpmIxSk37kIuKMzcwP9BWhFF0mx6mCUEaxv GdAQ9Va/uXB2TOyhLCGXhlf8uCwxcIyrOlhi2bK6ZIwwovyjjh7GCRnm8cP8ohDCJlDUpHkO pmU4tcapbZiBrFaFAahxPMjwK9GJ3JY0lx63McgCEIwm6txNcMnVX5Y3HeW5Wo8DtmeM3Xaj JLFaBXIhEfoNHMfDON6UGiXFeR8S9W8dpaX8XEwzPUjZyOG2LvOMAEPXx+kB9mZPTogong8L ekL1HZHSY4OYffzQy5fVE+woHAMADkrmuosGkTRCP4IQHXOagoax/Dox01lKTLnlUL1iWWQj fRaFXVKxEc2PF1RZUpoO/IQYFB1twcaF2ibT3TlGolbmb3qUYBo/Apl5GJUj/xOWwrbikD+C i+vx8yuFUlulbS9Ht+3z1dFjBUDbtZ4Bdy/1heNpA9xORiRs+M4GyTil33pnBXEZp29nh7ev 4VJ96sVvnQFzls3motvG+pq/c37Ms1gYayeCzA2iCDuKx6ZkybHg7IzNEduqZQ4bkaBpnEt+ vwE3Gg5l4dAUFWAs9qY13nyBANQ282FNctziEHCUJZ/Map6TdzHWO6hU1HuvmlwcJSFCOey8 yhkt386E6KfVYzrIhwTtabg+DLyMZK40Rop1VcU7Nx0M In-Reply-To: <20250508143451.GQ3865826@google.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - vern.gendns.com X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - lechnology.com X-Get-Message-Sender-Via: vern.gendns.com: authenticated_id: davidmain+lechnology.com/only user confirmed/virtual account not confirmed X-Authenticated-Sender: vern.gendns.com: davidmain@lechnology.com X-Source: X-Source-Args: X-Source-Dir: On 5/8/25 9:34 AM, Lee Jones wrote: > On Mon, 05 May 2025, Ivan Stepchenko wrote: > >> The copy_to_user() is annotated with __must_check, indicating that >> its return value must be checked by the caller. Currently, uleds_read() >> ignores it. If the userspace buffer is invalid and copy_to_user() fails, >> the userspace application may assume it has received fresh data, while >> in fact copying has failed. This can leave applications out of sync >> with the actual device state. >> >> Found by Linux Verification Center (linuxtesting.org) with SVACE. >> >> Fixes: e381322b0190 ("leds: Introduce userspace LED class driver") >> Signed-off-by: Ivan Stepchenko >> --- >> drivers/leds/uleds.c | 11 +++++++---- >> 1 file changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/leds/uleds.c b/drivers/leds/uleds.c >> index 374a841f18c3..41bfce43136c 100644 >> --- a/drivers/leds/uleds.c >> +++ b/drivers/leds/uleds.c >> @@ -147,10 +147,13 @@ static ssize_t uleds_read(struct file *file, char __user *buffer, size_t count, >> } else if (!udev->new_data && (file->f_flags & O_NONBLOCK)) { >> retval = -EAGAIN; >> } else if (udev->new_data) { >> - retval = copy_to_user(buffer, &udev->brightness, >> - sizeof(udev->brightness)); >> - udev->new_data = false; >> - retval = sizeof(udev->brightness); >> + if (copy_to_user(buffer, &udev->brightness, >> + sizeof(udev->brightness))) { > > This is not good. > > Please store the result into a variable and return that instead. Every other caller of copy_to_user() in the kernel I've seen ignores the actual return value and returns -EFAULT, so I thought this looked correct and it was just a quirk of copy_to_user(). > >> + retval = -EFAULT; >> + } else { >> + udev->new_data = false; >> + retval = sizeof(udev->brightness); >> + } >> } >> >> mutex_unlock(&udev->mutex); >> -- >> 2.39.5 >> >