From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lf1-f50.google.com (mail-lf1-f50.google.com [209.85.167.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 0F21A1A682E for ; Wed, 10 Jun 2026 01:04:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.50 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781053495; cv=none; b=pF1CiEWPPJYpm2/3idlGG3mSit5l4silSzBnu69r59mYu67aUoR1ceQIk0cKOQ+8AIwnybPX4T3eYvaLid/yaBbmji8YYhYEAyz6cDMr38oqurVrNw5p3/zdGJ0++FHog0dJd2JqeDNpoYBfYwjY8tsM8FtSUP4uAI4kLScXJL4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781053495; c=relaxed/simple; bh=BgouVNDaMkUocleo1nUzgz4x0HgWmDSLRPJA2tZ6RAA=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=KEZg6fX3JbhCRoCB++J4ttBJeS0J6OuPNBLrFHKd9p8IBJh3viR6vno8XwEJxZdWgSI7EXI8PuxFU8vqis2DrggJK9KF83JT3ahg85mAwNzUyUC6eDCyzZ0l9BqoyrupSJQ9raXnl8nqFN9jwlV6CihuMjT7YBgIqip1a/xf1Xc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=dubeyko.com; spf=pass smtp.mailfrom=dubeyko.com; dkim=pass (2048-bit key) header.d=dubeyko-com.20251104.gappssmtp.com header.i=@dubeyko-com.20251104.gappssmtp.com header.b=OpBsK6sl; arc=none smtp.client-ip=209.85.167.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=dubeyko.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=dubeyko.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=dubeyko-com.20251104.gappssmtp.com header.i=@dubeyko-com.20251104.gappssmtp.com header.b="OpBsK6sl" Received: by mail-lf1-f50.google.com with SMTP id 2adb3069b0e04-5aa68e66128so6353273e87.2 for ; Tue, 09 Jun 2026 18:04:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dubeyko-com.20251104.gappssmtp.com; s=20251104; t=1781053491; x=1781658291; darn=vger.kernel.org; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:from:to:cc:subject :date:message-id:reply-to; bh=3+IHp2l2rrkkQsrMXx/C+M6l/022rECrJz/KtdXmcpQ=; b=OpBsK6sllyNkGqKRBfe4snDCkwvgWNNmmjknzfUGHs9KaUxPVjF9hckXGIf0OhYP9A UaSy3d6UiqtuaskdwxT5WK2Wz4gSCUfTCUiludAJyBD6pIlylyHXM5mFDyV04CFfWJF3 WX3Xns8gpbWl+XDEDklQXRN6bvaiZink8eXTCEgQxAo4S5wOyiLI6SbfMcmXQZP70ssy g4Q/2yOkjx68lPa4peMvIM4I/wa9k/oOE/+uQRZ+xoJgB7qxkU669r6luy2O0LJodNzy xpqZ94e1bUIAILuG+QCrcujFPQbxyDyyP+Cmekm3gL1L+zXBz6Dzue+hFIFmRv6qSHEg zwSA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1781053491; x=1781658291; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=3+IHp2l2rrkkQsrMXx/C+M6l/022rECrJz/KtdXmcpQ=; b=EUEpE55peEqSDrUa0L8SUQc+T1XGfw9C03M9I2Lil9YI2E0u3n0EmlI+rNlUPEdvj9 XuXFm3U9kxUNlWMxzgR1+Ga0QV//4XkI94mfrKkrtVLXmvJVj9gnOvO/Q8lC8BZzfNqC eXNPlVJySLKEEFgXjPirFc2hz/s66s933smsoMUmrgG4MaM6IURtww4kWXyvaAOecRnM HbcWFy7PMhPLQDB5D4NtbqtYs6gCC2UNkXUZRBDBkBtC0fooVINAnZpto2DnhWyKL/p3 0XG4VDu9TKTjletxgwMePivgd8GEGKPIZH6p4+opUtpw9DoHa3PuHxkG2cYPhSGmDQmw 31mg== X-Forwarded-Encrypted: i=1; AFNElJ/gxdA5HKxL0i/XncYEf2NhUqy0VFF33RuVeXa8tDuFYncrAiLOnC8KP73i0Y9Q9WxrLyqL9zg/hR0dXX4=@vger.kernel.org X-Gm-Message-State: AOJu0Yzi8PzxSvn1dKDzaJMxtaEhPzqdqjR5o28aLUtkazMZAHHn1VxQ w48I5SduzqE+tV1uIgNRDLyfVS8KVxfdxo71WD1xQ2vE8RItitoZ50jMVHD704LjHazfJYS7e4e vWjIHdSUZ6Q== X-Gm-Gg: Acq92OGvdwwUuJYx2WpbkMhJwfGA4jkKxV08xRjvGQrsnIDeuknzistTnVjfhpiFOvA Vn8+EsxOWrjGLo0lq+Lf02P4HPP+hforjrOWXZOcO97qhFWbApT3f5KpInAkak0TxMnEsuPD9ox 0zzJ7rmBO4GVoyT7GfrJBn4pYWdTgG2wXK2K/aNc3C4pU5+dv+49svggrwnArDwaVhVyLA993JF iO6sIusLgxUC59f/GkYYONoww5R+JpbirGuhhhJNx3dLmH/11sl144rOzC34OyB3CW/QNN/dCMs XwbUlmEVLzcYwm61EEgrTSy0CFL6xGdmqLr5i5YnT3pzfCzstsHDcfBhkMIHjktu7k8pPyEsWHr BQyxkQCnEDFsIH+JXfzlMe+5vXlMk9AVY9NiyIrrgCMi/MlSg25pG7WAfBAFQwlPPEQhBl7q79x WSL9FjcOOxKx2FjJ/xNjEMRUvvivOmPEnYTMEKhZ8Nd4ZLiEbRMc6pmnaZFfng3cQLyAhHSAnmf mO8dTafEjYcenr+kYGPALrNhWioMxDIkwXgA8+RlhiDih6UH66k29BepRtHh4sQScGZcmHbdABr ew== X-Received: by 2002:a05:6512:108c:b0:5aa:6f5d:ee0b with SMTP id 2adb3069b0e04-5aa87b8b34cmr6193269e87.2.1781053490709; Tue, 09 Jun 2026 18:04:50 -0700 (PDT) Received: from [192.168.1.246] (broadband-95-84-186-252.ip.moscow.rt.ru. [95.84.186.252]) by smtp.gmail.com with ESMTPSA id 2adb3069b0e04-5aa7b97ac3fsm5007983e87.42.2026.06.09.18.04.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 09 Jun 2026 18:04:49 -0700 (PDT) Message-ID: <45e421ab38cfdbddbc0d34970a2db94902ad634b.camel@dubeyko.com> Subject: Re: [PATCH next] fs/hfsplus/xattr: Use memcpy() and strscpy() to build xattr_name From: Viacheslav Dubeyko To: david.laight.linux@gmail.com, Kees Cook , linux-hardening@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Arnd Bergmann , John Paul Adrian Glaubitz , Yangtao Li Date: Tue, 09 Jun 2026 18:04:46 -0700 In-Reply-To: <20260608095523.2606-39-david.laight.linux@gmail.com> References: <20260608095523.2606-39-david.laight.linux@gmail.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.60.1 (by Flathub.org) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 On Mon, 2026-06-08 at 10:55 +0100, david.laight.linux@gmail.com wrote: > From: David Laight >=20 > xattr_name is kmalloc()ed at the (assumed) maximal size and then the > prefix > and name concatenated together. > Use memcpy() for the prefix - its length is passed and strscpy() for > the > name to ensure it really doesnt overflow. >=20 > Prior to bf29e886b242c the buffers were smaller and on-stack. > (But I cant see the copy in the old code.) > I am also not sure why the buffer isnt created "just long enough". What do you mean by "the buffer isn't created just long enough"? And what is your vision of correct implementation of the logic? Thanks, Slava. >=20 > Signed-off-by: David Laight > --- > This is one of a group of patches that remove potentially unbounded > strcpy() calls. >=20 > They are mostly replaced by strscpy() or, when strlen() has just been > called, with memcpy() (usually including the '\0'). >=20 > Calls with copy string literals into arrays are left unchanged. > They are safe and easily detected as such. >=20 > The changes were made by getting the compiler to detect the calls and > then fixing the code by hand. >=20 > Note that all the changes are only compile tested. >=20 > Some Makefiles were changed to allow files to contain strcpy(). > As well as 'difficult to fix' files, this included 'show' functions > as they really need to use sysfs_emit() or seq_printf(). >=20 > All the patches are being sent individually to avoid very long cc > lists. > Apologies for the terse commit messages and likely unexpected tags. > (There are about 100 patches in total.) >=20 > =C2=A0fs/hfsplus/xattr.c | 12 ++++++------ > =C2=A01 file changed, 6 insertions(+), 6 deletions(-) >=20 > diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c > index 452a1f9becb2..0b3dd48c28c9 100644 > --- a/fs/hfsplus/xattr.c > +++ b/fs/hfsplus/xattr.c > @@ -550,8 +550,8 @@ int hfsplus_setxattr(struct inode *inode, const > char *name, > =C2=A0 xattr_name =3D kmalloc(xattr_name_len, GFP_KERNEL); > =C2=A0 if (!xattr_name) > =C2=A0 return -ENOMEM; > - strcpy(xattr_name, prefix); > - strcpy(xattr_name + prefixlen, name); > + memcpy(xattr_name, prefix, prefixlen); > + strscpy(xattr_name + prefixlen, name, xattr_name_len - > prefixlen); > =C2=A0 res =3D __hfsplus_setxattr(inode, xattr_name, value, size, > flags); > =C2=A0 kfree(xattr_name); > =C2=A0 > @@ -698,6 +698,7 @@ ssize_t hfsplus_getxattr(struct inode *inode, > const char *name, > =C2=A0 void *value, size_t size, > =C2=A0 const char *prefix, size_t prefixlen) > =C2=A0{ > + size_t xattr_name_len =3D NLS_MAX_CHARSET_SIZE * > HFSPLUS_ATTR_MAX_STRLEN + 1; > =C2=A0 int res; > =C2=A0 char *xattr_name; > =C2=A0 > @@ -705,13 +706,12 @@ ssize_t hfsplus_getxattr(struct inode *inode, > const char *name, > =C2=A0 inode->i_ino, name ? name : NULL, > =C2=A0 prefix ? prefix : NULL); > =C2=A0 > - xattr_name =3D kmalloc(NLS_MAX_CHARSET_SIZE * > HFSPLUS_ATTR_MAX_STRLEN + 1, > - =C2=A0=C2=A0=C2=A0=C2=A0 GFP_KERNEL); > + xattr_name =3D kmalloc(xattr_name_len, GFP_KERNEL); > =C2=A0 if (!xattr_name) > =C2=A0 return -ENOMEM; > =C2=A0 > - strcpy(xattr_name, prefix); > - strcpy(xattr_name + prefixlen, name); > + memcpy(xattr_name, prefix, prefixlen); > + strscpy(xattr_name + prefixlen, name, xattr_name_len - > prefixlen); > =C2=A0 > =C2=A0 res =3D __hfsplus_getxattr(inode, xattr_name, value, size); > =C2=A0 kfree(xattr_name);