From: Heiko Carstens <hca@linux.ibm.com>
To: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Thorsten Blum <thorsten.blum@linux.dev>,
Gerald Schaefer <gerald.schaefer@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Christian Borntraeger <borntraeger@linux.ibm.com>,
Sven Schnelle <svens@linux.ibm.com>,
linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] s390/cmm: Account for NUL when calculating 'len' in cmm_timeout_handler
Date: Mon, 22 Dec 2025 11:59:55 +0100 [thread overview]
Message-ID: <20251222105955.16440B52-hca@linux.ibm.com> (raw)
In-Reply-To: <4b00c8c6-e50b-454d-985d-0a5e2c3d77e9-agordeev@linux.ibm.com>
On Thu, Dec 18, 2025 at 08:57:24AM +0100, Alexander Gordeev wrote:
> On Mon, Dec 15, 2025 at 01:22:14PM +0100, Thorsten Blum wrote:
> > When the input length 'lenp' equals sizeof(buf), the current code copies
> > all 64 bytes, but then immediately overwrites the last byte with a NUL
> > terminator. Limit the number of bytes to copy to 'sizeof(buf) - 1' to
> > reserve space for the NUL terminator.
>
> I see you point, but can not see much of the benefit. Besides,
> to me buf[len] = '\0' rings like a past-end-of-the-buffer access
> (although it is not, it feels like that on a cursory look).
>
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > ---
> > arch/s390/mm/cmm.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/s390/mm/cmm.c b/arch/s390/mm/cmm.c
> > index eb7ef63fab1e..06512bc178a5 100644
> > --- a/arch/s390/mm/cmm.c
> > +++ b/arch/s390/mm/cmm.c
> > @@ -311,9 +311,9 @@ static int cmm_timeout_handler(const struct ctl_table *ctl, int write,
> > }
> >
> > if (write) {
> > - len = min(*lenp, sizeof(buf));
> > + len = min(*lenp, sizeof(buf) - 1);
> > memcpy(buf, buffer, len);
> > - buf[len - 1] = '\0';
> > + buf[len] = '\0';
Well, I don't see any point at all: the compiler is able to tell the
same and may or may not generate code accordingly to what this patch
tries to improve. This patch is pointless.
prev parent reply other threads:[~2025-12-22 11:00 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-15 12:22 [PATCH] s390/cmm: Account for NUL when calculating 'len' in cmm_timeout_handler Thorsten Blum
2025-12-18 7:57 ` Alexander Gordeev
2025-12-22 10:59 ` Heiko Carstens [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251222105955.16440B52-hca@linux.ibm.com \
--to=hca@linux.ibm.com \
--cc=agordeev@linux.ibm.com \
--cc=borntraeger@linux.ibm.com \
--cc=gerald.schaefer@linux.ibm.com \
--cc=gor@linux.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=svens@linux.ibm.com \
--cc=thorsten.blum@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox