public inbox for linux-fbdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Artem Lytkin <iprintercanon@gmail.com>
Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>,
	Teddy Wang <teddy.wang@siliconmotion.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-fbdev@vger.kernel.org, linux-staging@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/4] staging: sm750fb: add bounds checking to option parsing in lynxfb_setup()
Date: Wed, 4 Feb 2026 14:10:36 +0300	[thread overview]
Message-ID: <aYMpLEM7ZfXglGbc@stanley.mountain> (raw)
In-Reply-To: <20260204101536.3311-1-iprintercanon@gmail.com>

On Wed, Feb 04, 2026 at 10:15:33AM +0000, Artem Lytkin wrote:
> Replace strcat() with memcpy() and add explicit bounds checking on the
> remaining buffer space before each copy. The original code lacked any
> validation that the write position stays within the allocated buffer.
> 

This implies that there is a buffer overflow.  It's important to review
this sort of thing and add a Fixes tag if the buffer overflow is real.

In this case the code works ok as-is.  My main problem with the
original code is what you explained in the v1 commit, the strcat() was
just doing a memcpy().  It wasn't concatenating two strings.

Just resend the v1 patch with the following commit message:

  As part of kernel hardening, I am auditing calls to strcat().  This
  code works but it is a bit ugly.

  This function takes a string "options" and allocates "g_settings"
  which is large enough to hold a copy of "options".  It copies all the
  options from "options" to "g_settings" except "noaccel", "nomtrr" and
  "dual".  The new buffer is large enough to fit all the options so
  there is no buffer overflow in using strcat() here.

  However, using strcat() is misleading because "tmp" always points
  to the next unused character in the "g_settings" buffer and it's
  always the NUL character.  Use memcpy() instead to make the code
  easier to read.  This also removes an instance of strcat() which
  is a #NiceBonus.

regards,
dan carpenter


      parent reply	other threads:[~2026-02-04 11:10 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-04 10:15 [PATCH v2 1/4] staging: sm750fb: add bounds checking to option parsing in lynxfb_setup() Artem Lytkin
2026-02-04 10:15 ` [PATCH v2 2/4] staging: sm750fb: use strcmp() for exact option matching Artem Lytkin
2026-02-04 11:11   ` Dan Carpenter
2026-02-04 10:15 ` [PATCH v2 3/4] staging: sm750fb: remove debug prints and convert logging in sm750.c Artem Lytkin
2026-02-04 11:17   ` Dan Carpenter
2026-02-04 10:15 ` [PATCH v2 4/4] staging: sm750fb: remove debug prints and convert logging in sm750_hw.c Artem Lytkin
2026-02-04 11:39   ` Dan Carpenter
2026-02-04 11:10 ` Dan Carpenter [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=aYMpLEM7ZfXglGbc@stanley.mountain \
    --to=dan.carpenter@linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=iprintercanon@gmail.com \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=sudipm.mukherjee@gmail.com \
    --cc=teddy.wang@siliconmotion.com \
    /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