1. Patchew
  2. linux
  3. View series

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

Markus Elfring posted 1 patch 1 week, 5 days ago 
drivers/video/fbdev/arkfb.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH] fbdev: arkfb: Move a variable assignment behind a condition check in ics5342_init()
Posted by Markus Elfring 1 week, 5 days ago 
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()
Posted by Helge Deller 1 week ago 
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()
Posted by Markus Elfring 1 week ago 
>> 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()
Posted by Uwe Kleine-König 6 days, 23 hours ago 
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
Re: fbdev: arkfb: Move a variable assignment behind a condition check in ics5342_init()
Posted by Markus Elfring 6 days, 13 hours ago 
>> 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
Re: fbdev: arkfb: Move a variable assignment behind a condition check in ics5342_init()
Posted by Helge Deller 1 week ago 
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()
Posted by Markus Elfring 5 days, 11 hours ago 
>>>> 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.
Do any more code reviewers take another look at results from undefined behaviour sanitizers?

* https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#introduction

* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/dev-tools/ubsan.rst?h=v7.2-rc3#n6


See also:
https://stackoverflow.com/questions/79662603/why-do-compilers-not-warn-about-this-null-dereferencing-even-when-they-detect-i#answer-79663272

Regards,
Markus
Re: fbdev: arkfb: Move a variable assignment behind a condition check in ics5342_init()
Posted by Markus Elfring 6 days, 14 hours ago 
>> 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

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.