[PATCH] fbdev: arkfb: Move a variable assignment behind a condition check in ics5342_init()

[PATCH] fbdev: arkfb: Move a variable assignment behind a condition check in ics5342_init()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 13 Jul 2026 12:12:52 +0200

The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the function “ics5342_init”.

Thus avoid the risk for undefined behaviour by moving the assignment
for the variable “info” behind a condition check.

This issue was detected by using the Coccinelle software.

Fixes: ede481f6dad47d40b7e561cfbc6c04286a9faf1a ("fbdev: arkfb: Cast ics5342_init() allocation type")
Cc: stable@vger.kernel.org
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/video/fbdev/arkfb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
index 195dbf4a5142..9658f407b79a 100644
--- a/drivers/video/fbdev/arkfb.c
+++ b/drivers/video/fbdev/arkfb.c
@@ -432,11 +432,12 @@ static struct dac_ops ics5342_ops = {
 static struct dac_info * ics5342_init(dac_read_regs_t drr, dac_write_regs_t dwr, void *data)
 {
 	struct ics5342_info *ics_info = kzalloc_obj(struct ics5342_info);
-	struct dac_info *info = &ics_info->dac;
+	struct dac_info *info;
 
 	if (!ics_info)
 		return NULL;
 
+	info = &ics_info->dac;
 	info->dacops = &ics5342_ops;
 	info->dac_read_regs = drr;
 	info->dac_write_regs = dwr;
-- 
2.55.0

Re: [PATCH] fbdev: arkfb: Move a variable assignment behind a condition check in ics5342_init()

[Helge Deller]

On 7/13/26 12:18, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 13 Jul 2026 12:12:52 +0200
> 
> The address of a data structure member was determined before
> a corresponding null pointer check in the implementation of
> the function “ics5342_init”.
> 
> Thus avoid the risk for undefined behaviour by moving the assignment
> for the variable “info” behind a condition check.
> 
> This issue was detected by using the Coccinelle software.

There is no "risk" here.
It just adds an offset to a potential NULL value (which isn't then used afterwards).

Helge 
> Fixes: ede481f6dad47d40b7e561cfbc6c04286a9faf1a ("fbdev: arkfb: Cast ics5342_init() allocation type")
> Cc: stable@vger.kernel.org
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>   drivers/video/fbdev/arkfb.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
> index 195dbf4a5142..9658f407b79a 100644
> --- a/drivers/video/fbdev/arkfb.c
> +++ b/drivers/video/fbdev/arkfb.c
> @@ -432,11 +432,12 @@ static struct dac_ops ics5342_ops = {
>   static struct dac_info * ics5342_init(dac_read_regs_t drr, dac_write_regs_t dwr, void *data)
>   {
>   	struct ics5342_info *ics_info = kzalloc_obj(struct ics5342_info);
> -	struct dac_info *info = &ics_info->dac;
> +	struct dac_info *info;
>   
>   	if (!ics_info)
>   		return NULL;
>   
> +	info = &ics_info->dac;
>   	info->dacops = &ics5342_ops;
>   	info->dac_read_regs = drr;
>   	info->dac_write_regs = dwr;

Re: fbdev: arkfb: Move a variable assignment behind a condition check in ics5342_init()

[Markus Elfring]

>> The address of a data structure member was determined before
>> a corresponding null pointer check in the implementation of
>> the function “ics5342_init”.
>>
>> Thus avoid the risk for undefined behaviour by moving the assignment
>> for the variable “info” behind a condition check.
>>
>> This issue was detected by using the Coccinelle software.
> 
> There is no "risk" here.
> It just adds an offset to a potential NULL value (which isn't then used afterwards).
Does your understanding of programming language details differ from the view of
SEI CERT C Coding Standard (from the Carnegie Mellon University)?
https://cmu-sei.github.io/secure-coding-standards/sei-cert-c-coding-standard/rules/expressions-exp/exp34-c/

Regards,
Markus

Re: fbdev: arkfb: Move a variable assignment behind a condition check in ics5342_init()

[Helge Deller]

On 7/18/26 22:34, Markus Elfring wrote:
>>> The address of a data structure member was determined before
>>> a corresponding null pointer check in the implementation of
>>> the function “ics5342_init”.
>>>
>>> Thus avoid the risk for undefined behaviour by moving the assignment
>>> for the variable “info” behind a condition check.
>>>
>>> This issue was detected by using the Coccinelle software.
>>
>> There is no "risk" here.
>> It just adds an offset to a potential NULL value (which isn't then used afterwards).
> Does your understanding of programming language details differ from the view of
> SEI CERT C Coding Standard (from the Carnegie Mellon University)?
> https://cmu-sei.github.io/secure-coding-standards/sei-cert-c-coding-standard/rules/expressions-exp/exp34-c/
My statement still stands.
Try to find the difference between the code and the examples on that website yourself.
Tip: The relevant part is the "&" and in doubt look at the generated assembly code.

I will not discuss this further.

Helge

Re: fbdev: arkfb: Move a variable assignment behind a condition check in ics5342_init()

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1146 bytes --]

Hallo Markus,

On Sat, Jul 18, 2026 at 10:34:33PM +0200, Markus Elfring wrote:
> >> The address of a data structure member was determined before
> >> a corresponding null pointer check in the implementation of
> >> the function “ics5342_init”.
> >>
> >> Thus avoid the risk for undefined behaviour by moving the assignment
> >> for the variable “info” behind a condition check.
> >>
> >> This issue was detected by using the Coccinelle software.
> > 
> > There is no "risk" here.
> > It just adds an offset to a potential NULL value (which isn't then used afterwards).
> Does your understanding of programming language details differ from the view of
> SEI CERT C Coding Standard (from the Carnegie Mellon University)?
> https://cmu-sei.github.io/secure-coding-standards/sei-cert-c-coding-standard/rules/expressions-exp/exp34-c/

We recently discussed a similar case where several people told you that
the "problem" you fixed wasn't actually a problem. This patch is in the
same category and your reference doesn't match the code touched here.

Please stop to absorb maintainer attention with useless stuff.

Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: fbdev: arkfb: Move a variable assignment behind a condition check in ics5342_init()

[Markus Elfring]

>> Does your understanding of programming language details differ from the view of
>> SEI CERT C Coding Standard (from the Carnegie Mellon University)?
>> https://cmu-sei.github.io/secure-coding-standards/sei-cert-c-coding-standard/rules/expressions-exp/exp34-c/
> My statement still stands.
> Try to find the difference between the code and the examples on that website yourself.
> Tip: The relevant part is the "&"

Which functionality would you expect for the operator “address of”?

Is it applied only after a pointer dereference attempt in this case?
https://en.cppreference.com/c/language/operator_member_access


>                                   and in doubt look at the generated assembly code.

Can development interests grow also according to another clarification approach?

Does &((struct name *)NULL -> b) cause undefined behaviour in C11?
https://stackoverflow.com/questions/26906621/does-struct-name-null-b-cause-undefined-behaviour-in-c11

Regards,
Markus

Re: fbdev: arkfb: Move a variable assignment behind a condition check in ics5342_init()

[Markus Elfring]

>> Does your understanding of programming language details differ from the view of
>> SEI CERT C Coding Standard (from the Carnegie Mellon University)?
>> https://cmu-sei.github.io/secure-coding-standards/sei-cert-c-coding-standard/rules/expressions-exp/exp34-c/
> 
> We recently discussed a similar case where several people told you that
> the "problem" you fixed wasn't actually a problem.

We came along possible adjustments according to an application of another
questionable sanity check.


>                                                    This patch is in the
> same category and your reference doesn't match the code touched here.

Will development interests grow if bug reports will follow by further known
source code analysis tools?
https://cmu-sei.github.io/secure-coding-standards/sei-cert-c-coding-standard/rules/expressions-exp/exp34-c/#automated-detection


> Please stop to absorb maintainer attention with useless stuff.
Do we need to start negotiations for allocation of a corresponding CVE ID
(as something happened for similar control flows before)?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/cve.rst?h=v7.2-rc3#n16

Regards,
Markus