From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (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 9443C36894F for ; Fri, 24 Apr 2026 19:39:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777059543; cv=none; b=K1vr9wb56KixBzN0X54d/lOzOxXAeStlejiGNNn8P7MhAS7xufly32XydBW/WPJoqw7r4mVA/Q+pFV6jvrxBcGzkBjnOWyMDVn0yGChvaYl5M+6xpymhfz32Nk7Bzo0gAXfwG3T9ynOpfEEyGH4sRXYpk6Ws2tgyonfA/LW2ukg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777059543; c=relaxed/simple; bh=lPqFX0Yy0XAE/Rw/BAATs7v0SQyiUopmPVCSq+WVYWw=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=qKmaKgfncZ4WgigWeSqtGG21960XNSX4e3M5MXNzkbr8uYg+as0eUQ4NFtKE0gAsmozrjLlfszk9JUEeU0e9NUR+cfRsF3yzNHYTkxPd0O5nziztPbubzJsTDyk0ib3fxcSGXE55xBHXkr0uxwZ3073M9HuwRogH4mCPoQJwfb0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=WsXb92nE; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=qEPdFLqk; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="WsXb92nE"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="qEPdFLqk" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1777059539; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=G7VaufB6w1dJvFuFr1EOIEp5Ep8hIo8a4pkkuPUO7RM=; b=WsXb92nEOJ9HkExeul423+sDA7os96dgPrkCXQmxveCuJb+wmOxRiD93otNnevNdHdLgCh dkBIYKELH5EYvDn8NYeO+VxkbbtNsTtiGxzDelYIAXX72ctbkvteghblFjV96DXY3QydUq CAhs06hS82sAwy/kAka5tJjX1E5QKbk= Received: from mail-oi1-f198.google.com (mail-oi1-f198.google.com [209.85.167.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-683-kuf6aZsaNVmygm60oQfmvA-1; Fri, 24 Apr 2026 15:38:58 -0400 X-MC-Unique: kuf6aZsaNVmygm60oQfmvA-1 X-Mimecast-MFC-AGG-ID: kuf6aZsaNVmygm60oQfmvA_1777059537 Received: by mail-oi1-f198.google.com with SMTP id 5614622812f47-47bd318565dso1368147b6e.1 for ; Fri, 24 Apr 2026 12:38:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1777059537; x=1777664337; 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=G7VaufB6w1dJvFuFr1EOIEp5Ep8hIo8a4pkkuPUO7RM=; b=qEPdFLqkQKnMCygqdPDxs0pjT5dW8R7Vka6thdHdveudGlN4UzQY9rEHbhy/Qo6f1S eW0FFClkUi2Vh01mrLMTOT6eEdP82seWaxfpjf9+jzobIE4hktIA/KKSeX7PWm0TBpRO jIaUiuSaAK3TYaLF5gGxdMGkp6H/lKOY6aU1rvKwimaQlWExj96/4hi5aX0Yl28smVVG DRZeH6KqQNkt5DVREpL6xbX7yl2iGjO/+uHqVBE4Bsn7zvZfqWfhItbJsPQnr/eIc5OR CRMCfKPGFsScH7tcygq33zsa53GWDg+C+WV8KtT1pseXztBAxnQ7oyUKjAjw8lWRkDMy niwg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777059537; x=1777664337; 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=G7VaufB6w1dJvFuFr1EOIEp5Ep8hIo8a4pkkuPUO7RM=; b=Zfj7U4PBBdzpPzWueP9b/OFXe909l3ELgtHsMdmPSbVn681LhHIe3O3Byadv8Q5meu TUj4DiL+/MVILIenxm3S1EfOh7ogsHlY6ulyqFYvd2mVOyySGiAtt+UuU7o/VijYdowA rDODszt+g2vcCJL20k/IVJYMmePk+5JvnmrkPQ7UaDqMJBqEDPDEPAkV2p3XKS16nD7B VWNLN/9qBFoae75Tl5gXTaMSybK9AA/Fx5HdPSA/ru1ObMgQvRXNgw3hVb9Q5owc5XOF LZPsLQRO0pwDRrExFxoxieJEhAHra80m/deIF3tYHdqufa1/DaQlM0YcjltLQvWV768H 2ftQ== X-Forwarded-Encrypted: i=1; AFNElJ9VqYjWFmk/g5mP2gBZ1ZGSbRAkL/jrAM2TgBipdN+KS+OniVC0tCITd3jtvm7sMcWTNLgSGHdo0FsJ3Wc=@vger.kernel.org X-Gm-Message-State: AOJu0YyINqPZcpEQeZ0A16zA4SlYy9Kz7O8JlzWBhQoQOFsQYWI4dynO Xj7+6cf3eT6FYNdtMvNJ/69NB6jbSPObH+3wCMPaCZuWZ/VxgBLx7Cnz5seuguoqNwNkR1P15+S Fa6gq9FItWhLHZIRM9qTgALEQuPDTHH9cTxNBmJYe4vmbN6nDWdEQs0sJGSj1IMSocI0drAWYOQ == X-Gm-Gg: AeBDieuOg8J2OVvP0RT+3EeVjuDWdMMou+Ubr8KDY1Z+e3WoemFb+H+ylYFXCtwk7FP csqODHQhIEB6Pyc1w1Yqo4cFHDla9BiBvS/ERLYzG0CFXrVOGYXLfNm8rkVjvyoXQP2JRdagBRA YqdSZIyy9ob9i01StTMGouKENAe+vSHCDAnLPvIkdBuhlzY2b9Lz9fOwrwcMXljSPCWuGCChznA Cp28zlGYTc9Ls1c+ggvStm7EuH4Q+0qRoPr+c31HowyQOEoOJiWu60Xkzypv9g1IonhRLJ3MCGw A10Z6teZSodPNYhbHs6eIy2PAMOUkjp/h/w39VyU6vqShTEyDPGuPZW8Gnhk8faMNEB8TXWbkzv KmjNBd3KgzDEMX44nyflQHbnK/bqY3K9vD2FchfXqDZDBxUgkNXOTLrjTTj+igl0= X-Received: by 2002:a05:6808:2e43:b0:479:f0a2:d23c with SMTP id 5614622812f47-479f0a2e5e2mr10848516b6e.27.1777059537132; Fri, 24 Apr 2026 12:38:57 -0700 (PDT) X-Received: by 2002:a05:6808:2e43:b0:479:f0a2:d23c with SMTP id 5614622812f47-479f0a2e5e2mr10848501b6e.27.1777059536668; Fri, 24 Apr 2026 12:38:56 -0700 (PDT) Received: from li-4c4c4544-0032-4210-804c-c3c04f423534.ibm.com ([2600:1700:6476:1430::29]) by smtp.gmail.com with ESMTPSA id 5614622812f47-479a02097b6sm16156094b6e.14.2026.04.24.12.38.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Apr 2026 12:38:56 -0700 (PDT) Message-ID: Subject: Re: [PATCH] hfsplus: replace unbounded sprintf() in hfsplus_{lookup,link,unlink} From: Viacheslav Dubeyko To: Thorsten Blum , Viacheslav Dubeyko , John Paul Adrian Glaubitz , Yangtao Li Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Date: Fri, 24 Apr 2026 12:38:54 -0700 In-Reply-To: <20260424090633.307300-3-thorsten.blum@linux.dev> References: <20260424090633.307300-3-thorsten.blum@linux.dev> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.58.3 (3.58.3-1.fc43app2) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 On Fri, 2026-04-24 at 11:06 +0200, Thorsten Blum wrote: > While the current code works correctly, replace unbounded sprintf() > calls with the safer scnprintf() in hfsplus_lookup(), hfsplus_link(), > and hfsplus_unlink() to follow secure coding best practices. The patch makes sense. But I would like to request some refactoring work he= re. :) >=20 > Signed-off-by: Thorsten Blum > --- > fs/hfsplus/dir.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) >=20 > diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c > index 47194370c2c5..ff976995ef58 100644 > --- a/fs/hfsplus/dir.c > +++ b/fs/hfsplus/dir.c > @@ -98,7 +98,7 @@ static struct dentry *hfsplus_lookup(struct inode *dir,= struct dentry *dentry, > dentry->d_fsdata =3D (void *)(unsigned long)cnid; > linkid =3D > be32_to_cpu(entry.file.permissions.dev); > - str.len =3D sprintf(name, "iNode%d", linkid); > + str.len =3D scnprintf(name, sizeof(name), "iNode%d", linkid); > str.name =3D name; > err =3D hfsplus_cat_build_key(sb, fd.search_key, > HFSPLUS_SB(sb)->hidden_dir->i_ino, This piece of code looks like a static inline function. Could we rework thi= s piece of code? char name[32]; This declaration looks not very well. I think we need to introduce some nam= ed constant for hardcoded 32 value. Could you please rework it? > @@ -322,7 +322,7 @@ static int hfsplus_link(struct dentry *src_dentry, st= ruct inode *dst_dir, > get_random_bytes(&id, sizeof(cnid)); > id &=3D 0x3fffffff; > str.name =3D name; > - str.len =3D sprintf(name, "iNode%d", id); > + str.len =3D scnprintf(name, sizeof(name), "iNode%d", id); > res =3D hfsplus_rename_cat(inode->i_ino, > src_dir, &src_dentry->d_name, > sbi->hidden_dir, &str); The same issue with name declaration here: char name[32]; Also, I think it makes sense to initialize the local variables: char name[32] =3D {0}; Maybe it makes sense to introduce some small function that execute this: str.len =3D sprintf(name, "iNode%d", linkid); str.name =3D name; Something like this: str =3D hfsplus_qstr_init(); What do you think? > @@ -393,7 +393,7 @@ static int hfsplus_unlink(struct inode *dir, struct d= entry *dentry) > if (inode->i_ino =3D=3D cnid && > atomic_read(&HFSPLUS_I(inode)->opencnt)) { > str.name =3D name; > - str.len =3D sprintf(name, "temp%llu", inode->i_ino); > + str.len =3D scnprintf(name, sizeof(name), "temp%llu", inode->i_ino); > res =3D hfsplus_rename_cat(inode->i_ino, > dir, &dentry->d_name, > sbi->hidden_dir, &str); The same issues here too. Thanks, Slava. [1] https://elixir.bootlin.com/linux/v7.0/source/fs/hfsplus/dir.c#L87