Linux kernel staging patches
 help / color / mirror / Atom feed
From: Dan Carpenter <error27@gmail.com>
To: moonafterrain@outlook.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, Yuhao Jiang <danisjiang@gmail.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH] staging: sm750fb: do not program the PLL from an uninitialized value
Date: Mon, 17 Aug 2026 12:59:09 +0300	[thread overview]
Message-ID: <aoLbbZ--FrkVN3SU@stanley.mountain> (raw)
In-Reply-To: <20260817-sm750-fixes-v1-1-978c617e5a20@outlook.com>

On Mon, Aug 17, 2026 at 05:13:53PM +0800, Junrui Luo via B4 Relay wrote:
> From: Junrui Luo <moonafterrain@outlook.com>
> 
> sm750_calc_pll_value() writes pll->M, N, OD and POD only when its search
> loop finds a divider combination with 0 < M < 256, and returns 0 when
> there is none. ddk750_set_mode_timing() discards that return value and
> calls program_mode_registers() regardless, so sm750_format_pll_reg()
> reads the four members uninitialized and pokes them into PANEL_PLL_CTRL
> or CRT_PLL_CTRL. Nothing bounds var->pixclock on the way in, so a mode
> set can ask for a clock the loop cannot represent.
> 
> Consume the return value and reject the mode; hw_sm750_crtc_set_mode()
> already propagates a non-zero return. Initialize the structure as well:
> sm750_calc_pll_value() returns early for SM750LE without writing the
> members, and returns non-zero on that path.
> 
> Fixes: 81dee67e215b ("staging: sm750fb: add sm750 to staging")
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Assisted-by: Claude:claude-opus-5
> Cc: stable@vger.kernel.org
> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
> ---

Greg is not taking AI patches unless they can be tested.
https://lore.kernel.org/all/2026080354-skater-urgent-31b2@gregkh/

I kind of hate AI commit messages...  They are so verbose, confident
and reasonable sounding.  But they don't answer any of the real
questions I want to know.  How did Yuhao Jiang find this bug?  What
did the symptoms look like to a user?  Are there ways we could
improve our QC process to prevent this sort of bug in the future?

Probably the answer is that the bug was detected with AI and we
have no idea what the symptoms look like.  Everyone sane automatically
initializes variables to zero so probably there are no symptoms.

So the problem is that the user inputs invalid var->pixclock, and
it leads to an uninitialized variable usage.  This patch addresses
it by initializing he variable to zero and checking if
sm750_calc_pll_value() returns an error code.  Either approach on
its own would would fix the problem, hopefully right?  So it's a belt
and suspenders approach.  But isn't the real solution to reject
invalid pixclocks in lynxfb_ops_check_var()?

We're not going to apply this patch because it hasn't been tested.
Probably we should invent a new tag so we can create a TODO list
of rejected AI patches.

KTODO: investigate unintialized variables in sm750fb found by AI

regards,
dan carpenter


  reply	other threads:[~2026-08-17  9:59 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17  9:13 [PATCH] staging: sm750fb: do not program the PLL from an uninitialized value Junrui Luo via B4 Relay
2026-08-17  9:59 ` Dan Carpenter [this message]
2026-08-17 10:49   ` Yuhao Jiang
2026-08-17 11:13     ` Dan Carpenter
2026-08-17 11:27       ` Yuhao Jiang

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=aoLbbZ--FrkVN3SU@stanley.mountain \
    --to=error27@gmail.com \
    --cc=danisjiang@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=moonafterrain@outlook.com \
    --cc=stable@vger.kernel.org \
    --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