From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) (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 82EA02F5A2D for ; Wed, 19 Aug 2026 12:43:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.54 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787143399; cv=none; b=UnrLeQcasTBQlQ0k1WrHuHlI8NidBk4M6ghgKmjdwI5knZtdUGxtoJvHpnRJcLux3qqMpQl1/Ko7qjxbO/50ScZ3R+t6wEC2m9IduNjTlLB+oQFQs1lKCdNq6xi1/bO5xk/qtXu8FLD6ey2qsTQvQELqdHOpZwnCtHSim0oPaLg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787143399; c=relaxed/simple; bh=8NSAxoKdj2CImo9K22dfQYC4bFvoKWOqEIYItODfF5s=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=qFXas+cj3EVr6AYqclz0Ec1YWTbqUQ75WofKDwRm9gej1IiTvfG9+lh3Lgs8JI8LyQNm7XKQgZ6f8CZ+7m0dXd4inAzwHKVxB7lRvd/+e2VphGYiwhgZCgwyjZ75GYoSmCcTuZ9zpUdczUSQW2pz8ab7oT6IjBRv3nvl9lPM+kc= 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=IvmU4GOx; arc=none smtp.client-ip=209.85.128.54 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="IvmU4GOx" Received: by mail-wm1-f54.google.com with SMTP id 5b1f17b1804b1-499840a2575so7605865e9.3 for ; Wed, 19 Aug 2026 05:43:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1787143396; x=1787748196; darn=vger.kernel.org; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=4s1/nRtlK2r1n/sIa7tXaH0u1v7OW/0NCvTyRnewdHY=; b=IvmU4GOx5uxjbcrNTwDxpnLcX/wtGyNcGu1+KwW44C11Msd9Yp/Xhy2CW98OsL05bE jNgUpxTiwHxZ9SW6G52rINZPopaLimDByK2RofILJlRJz9MZ8qtJuGU/77ws9bFy2WiX FlOLpG40e2cqKh5UGCIcVhNWOrTGFhouWrlUofeo/wxmBm+lmITAsvzolAf7F5ytzYaJ Wqb9n3iZ8tsCg6YKMKeoX6QwwH1YpqqR1dwuXd06OAiuc6Gszak2qOOK9ZABnvt7hT5X jGCNgz/uRuPl4qMVtUsMYJDxFte/xw2uaASryUZbkIH+saveyAJ3X7dJB4KFBEUP2qP9 6E/g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1787143396; x=1787748196; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to: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=4s1/nRtlK2r1n/sIa7tXaH0u1v7OW/0NCvTyRnewdHY=; b=KrCQuGpGDhi0d13DxIChH886U9q4zDAp/LwtqVVE5Bk2Rr4Rt4i2KL+yiC2Dt6M7og TtWrRw7FDAINK5alSdg/Qg/ikrRXx4IDVMLid18fz9wTn6U18OKHo3TtsHL+18aI00W/ +Kdw8P0FcTGQosi/1pnAvdT8ye1dDo9EKBz053daEXIHYK4T1IAK0I6FskC3R8KKe6Ad VK/QAZRKBePjY/MX6HEnuU214klAjaOWQKBZIdVwH/1yqNAS6JXQe4a2czmm3+Y0f7TY iClHuxCTwobHLpF757UmBgAJsuWPsbqh4TEozuzq3vmXyxHA0w4GUr5cvuKrngt9wrNB /R7w== X-Forwarded-Encrypted: i=1; AHgh+RpGTGByy+Wq5d89Iw/YgHvnPX/lgh+vXGYTRKDX+rqGMpTdu6DhlTmZkRTn8I4An83eO0LNYwWoVreuMyv9xvbIWxXy@vger.kernel.org X-Gm-Message-State: AOJu0Yzy37vFiKKB2/baUcLL1z1ES7DK3uEIZKrVBz/33TziDSxXWdtM XmM9deni3B0PMY4C/HFSHSGPkL3uWkwB60iNbwprbCHM9iVNpZCWJgUq X-Gm-Gg: AR+sD11zMbo5I1iyQD2n5hiUOKzH10ojqS9IVz7rBEDXlsM92Jb3HQPznOeuCxZklbx uXd521SdEmseFnYTONFpwmnWzNb6PHJCsO7sOWTU9ZbmronO2YOke4eFvC2KcxLNpaptoNa8X+A cOqaeLqdscvOM33vwsIt848Ea/N1Whk8NANnxUy9YOOVe5XA4Zaj8VCHLOGD9E6aOuwF/XpIVDs pFSVaIEMbt7DNvCaUm6KrYfWvMZ94NPB/sLkmb3PBz3TdT26kKtLqeDK5B7TziyP/4i1NFQyjq9 /rpYa9phDPX7r01jIzMYTsyG6DKTO/VBOeviRIbuEb7RJ89u40YlyzOQ4nf4G8yM3/c+VNnbLCu TGjJCfzY4QILw5RH0Z4EVHRHbXnPe8t2p9Tf6NU/fyaVcTcTqO9bWaW1sZcgXLGaZBsS/pTNVve e5JZ4uKOUy6jBIhqMJWT8RQASnoJ1KgQZc6gFL8IXDW/NVLfKUM8o0yV41boGvsXohZGUFp6lON 9f6L0PKx0etJluHG75X4VWcLsGLmBUwFVQ= X-Received: by 2002:a05:600c:8b06:b0:495:5fdf:2075 with SMTP id 5b1f17b1804b1-499aa0dca0emr89422975e9.0.1787143395650; Wed, 19 Aug 2026 05:43:15 -0700 (PDT) Received: from pumpkin (82-69-66-36.dsl.in-addr.zen.co.uk. [82.69.66.36]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-499a9dffa59sm79314665e9.3.2026.08.19.05.43.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 19 Aug 2026 05:43:15 -0700 (PDT) Date: Wed, 19 Aug 2026 13:43:14 +0100 From: David Laight To: Thorsten Blum Cc: Ilpo =?UTF-8?B?SsOkcnZpbmVu?= , Mark Pearson , "Derek J. Clark" , Hans de Goede , stable@vger.kernel.org, Mark Pearson , platform-driver-x86@vger.kernel.org, LKML Subject: Re: [PATCH v2] platform/x86: think-lmi: Fix current password length check Message-ID: <20260819134314.54a6f835@pumpkin> In-Reply-To: References: <20260813082049.41209-2-thorsten.blum@linux.dev> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; arm-unknown-linux-gnueabihf) Precedence: bulk X-Mailing-List: platform-driver-x86@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Tue, 18 Aug 2026 15:39:35 +0200 Thorsten Blum wrote: > On Tue, Aug 18, 2026 at 04:20:10PM +0300, Ilpo J=C3=A4rvinen wrote: > > On Tue, 18 Aug 2026, Thorsten Blum wrote: =20 > > > On Tue, Aug 18, 2026 at 02:28:50PM +0300, Ilpo J=C3=A4rvinen wrote: = =20 > > > > On Thu, 13 Aug 2026, Thorsten Blum wrote: =20 > > > > > current_password_store() checks the password length before removi= ng the > > > > > trailing newline, which can reject valid passwords that are exact= ly =20 > > > > > ->maxlen bytes long. =20 > > > > >=20 > > > > > It also passes ->maxlen to strscpy(), which truncates passwords w= ithout > > > > > a newline. > > > > >=20 > > > > > Use strchrnul() to measure the password length up to the newline,= then > > > > > copy that many bytes and add a trailing NUL terminator. > > > > >=20 > > > > > Fixes: a40cd7ef22fb ("platform/x86: think-lmi: Add WMI interface = support on Lenovo platforms") > > > > > Cc: stable@vger.kernel.org > > > > > Signed-off-by: Thorsten Blum > > > > > --- > > > > > Changes in v2: > > > > > - Keep and reword the newline comment > > > > > - v1: https://lore.kernel.org/r/20260810132018.156868-3-thorsten.= blum@linux.dev/ > > > > > --- > > > > > drivers/platform/x86/lenovo/think-lmi.c | 8 ++++---- > > > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > >=20 > > > > > diff --git a/drivers/platform/x86/lenovo/think-lmi.c b/drivers/pl= atform/x86/lenovo/think-lmi.c > > > > > index e215e86e3db7..d0ceb6aaa69e 100644 > > > > > --- a/drivers/platform/x86/lenovo/think-lmi.c > > > > > +++ b/drivers/platform/x86/lenovo/think-lmi.c > > > > > @@ -438,14 +438,14 @@ static ssize_t current_password_store(struc= t kobject *kobj, > > > > > struct tlmi_pwd_setting *setting =3D to_tlmi_pwd_setting(kobj); > > > > > size_t pwdlen; > > > > > =20 > > > > > - pwdlen =3D strlen(buf); > > > > > + /* Strip newline; setting password won't work if one is present= . */ > > > > > + pwdlen =3D strchrnul(buf, '\n') - buf; > > > > > /* pwdlen =3D=3D 0 is allowed to clear the password */ > > > > > if (pwdlen && ((pwdlen < setting->minlen) || (pwdlen > setting-= >maxlen))) > > > > > return -EINVAL; > > > > > =20 > > > > > - strscpy(setting->password, buf, setting->maxlen); > > > > > - /* Strip out CR if one is present, setting password won't work = if it is present */ > > > > > - strreplace(setting->password, '\n', '\0'); > > > > > + memcpy(setting->password, buf, pwdlen); > > > > > + setting->password[pwdlen] =3D '\0'; =20 > > > >=20 > > > > Hi, > > > >=20 > > > > I don't understand why is this strscpy() -> memcpy() conversion req= uired? =20 > > >=20 > > > It's not required, strscpy() would also work. > > > =20 > > > > Wouldn't it work with: > > > >=20 > > > > strscpy(..., pwdlen); > > > >=20 > > > > ? =20 > > >=20 > > > However, strscpy(setting->password, buf, pwdlen) wouldn't work because > > > pwdlen is the number of characters to copy, but the destination buffer > > > size also needs to include room for the NUL terminator. Since there is > > > no NUL before pwdlen, it would copy only pwdlen - 1 characters and set > > > setting->password[pwdlen - 1] =3D '\0'. =20 > >=20 > > Okay, I was thinking in my mind if I've a off-by-one error in my=20 > > suggestion but didn't want to spend too much time on figuring it out. > > =20 > > > strscpy(setting->password, buf, pwdlen + 1) would work, but since we > > > already know that exactly pwdlen bytes need to be copied, memcpy() is > > > sufficient. =20 No that overwrites past the end out the output buffer. > >=20 > > It may work, but since you then go to nul terminate it yourself, I thin= k=20 > > using the existing function is way better than memcpy() + custom nul=20 > > termination code. =20 >=20 > I used memcpy() because we already determined the string length pwdlen, > which strscpy() would have to determine again internally. >=20 > The performance difference shouldn't matter here, so it's mostly a > matter of style or personal preference. I'm fine either way. Personally I'd always use memcpy() if the size is known. Whoever added the strscpy() should have used the size of the array not the soft bound for the password length (which is limited by the array size). In any case the code could be: pwdlen =3D count; if (pwdlen && buf[pwdlen - 1] =3D=3D '\n') pwdlen--; ... memcpy(setting->password, buf, pwdlen); setting->password[pwdlen] =3D 0; return count; } I'm sure there could be (might even be) a helper to copy some characters and append a '\0'. David >=20 > Thanks, > Thorsten >=20