[PATCH] staging: sm750fb: do not program the PLL from an uninitialized value

[PATCH] staging: sm750fb: do not program the PLL from an uninitialized value

[Junrui Luo via B4 Relay]

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>
---
 drivers/staging/sm750fb/ddk750_mode.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_mode.c b/drivers/staging/sm750fb/ddk750_mode.c
index 7163232c0701..5ff5226fe77a 100644
--- a/drivers/staging/sm750fb/ddk750_mode.c
+++ b/drivers/staging/sm750fb/ddk750_mode.c
@@ -205,12 +205,14 @@ static void program_mode_registers(struct mode_parameter *mode_param,
 
 int ddk750_set_mode_timing(struct mode_parameter *parm, enum clock_type clock)
 {
-	struct pll_value pll;
+	struct pll_value pll = {};
 
 	pll.input_freq = DEFAULT_INPUT_CLOCK;
 	pll.clock_type = clock;
 
-	sm750_calc_pll_value(parm->pixel_clock, &pll);
+	if (!sm750_calc_pll_value(parm->pixel_clock, &pll))
+		return -EINVAL;
+
 	if (sm750_get_chip_type() == SM750LE) {
 		/* set graphic mode via IO method */
 		outb_p(0x88, 0x3d4);

---
base-commit: f5bbbfec59b4e2fb7520a91de3df8a6174325d6a
change-id: 20260817-sm750-fixes-466849daffec

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>

Re: [PATCH] staging: sm750fb: do not program the PLL from an uninitialized value

[Dan Carpenter]

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

Re: [PATCH] staging: sm750fb: do not program the PLL from an uninitialized value

[Yuhao Jiang]

On Mon, Aug 17, 2026 at 4:59 AM Dan Carpenter <error27@gmail.com> wrote:
>
> 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

We're working on an LLM-assisted system for vulnerability discovery,
and this bug was found by the system and checked by me.

We proactively wrote this patch to push for a faster fix. This patch was
also assisted by AI but we manually reviewed it before submitting.

> 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()?

Yes.

>
> 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.

Since the bug is valid, and we lack the hardware to test it, leaving it
on the KTODO list is fine.

>
> KTODO: investigate unintialized variables in sm750fb found by AI
>
> regards,
> dan carpenter
>


--
Yuhao Jiang

Re: [PATCH] staging: sm750fb: do not program the PLL from an uninitialized value

[Dan Carpenter]

On Mon, Aug 17, 2026 at 05:49:33AM -0500, Yuhao Jiang wrote:
> On Mon, Aug 17, 2026 at 4:59 AM Dan Carpenter <error27@gmail.com> wrote:
> >
> > 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
> 
> We're working on an LLM-assisted system for vulnerability discovery,
> and this bug was found by the system and checked by me.
> 

I feel like this is always part of the story and should be told.
Regardless the firm rule is that everyone agrees on if you can't test it,
then that needs to be in the notes under the --- cut off.

(I sometimes skip this, but I've been doing this for a long long time
and everyone knows who I am.  I should follow the rules too.  #shame).

regards,
dan carpenter

Re: [PATCH] staging: sm750fb: do not program the PLL from an uninitialized value

[Yuhao Jiang]

On Mon, Aug 17, 2026 at 6:13 AM Dan Carpenter <error27@gmail.com> wrote:
>
> On Mon, Aug 17, 2026 at 05:49:33AM -0500, Yuhao Jiang wrote:
> > On Mon, Aug 17, 2026 at 4:59 AM Dan Carpenter <error27@gmail.com> wrote:
> > >
> > > 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
> >
> > We're working on an LLM-assisted system for vulnerability discovery,
> > and this bug was found by the system and checked by me.
> >
>
> I feel like this is always part of the story and should be told.
> Regardless the firm rule is that everyone agrees on if you can't test it,
> then that needs to be in the notes under the --- cut off.
>
> (I sometimes skip this, but I've been doing this for a long long time
> and everyone knows who I am.  I should follow the rules too.  #shame).
>
> regards,
> dan carpenter
>

Got it. We will follow this rule in the future. Thanks for the guidance.

-- 
Yuhao Jiang