Fwd: New Defects reported by Coverity Scan for Das U-Boot
Here's the latest report
---------- Forwarded message --------- From: scan-admin@coverity.com Date: Mon, Aug 21, 2023 at 4:30 PM Subject: New Defects reported by Coverity Scan for Das U-Boot To: tom.rini@gmail.com
Hi,
Please find the latest report on new defect(s) introduced to Das U-Boot found with Coverity Scan.
4 new defect(s) introduced to Das U-Boot found with Coverity Scan.
New defect(s) Reported-by: Coverity Scan Showing 4 of 4 defect(s)
** CID 464362: Integer handling issues (OVERFLOW_BEFORE_WIDEN) /drivers/video/pwm_backlight.c: 68 in set_pwm()
________________________________________________________________________________________________________ *** CID 464362: Integer handling issues (OVERFLOW_BEFORE_WIDEN) /drivers/video/pwm_backlight.c: 68 in set_pwm() 62 { 63 u64 width; 64 uint duty_cycle; 65 int ret; 66 67 if (priv->period_ns) {
CID 464362: Integer handling issues (OVERFLOW_BEFORE_WIDEN) Potentially overflowing expression "priv->period_ns * (priv->cur_level - priv->min_level)" with type "unsigned int" (32 bits, unsigned) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "u64" (64 bits, unsigned).
68 width = priv->period_ns * (priv->cur_level - priv->min_level); 69 duty_cycle = div_u64(width, 70 (priv->max_level - priv->min_level)); 71 ret = pwm_set_config(priv->pwm, priv->channel, priv->period_ns, 72 duty_cycle); 73 } else {
** CID 464361: Control flow issues (DEADCODE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log()
________________________________________________________________________________________________________ *** CID 464361: Control flow issues (DEADCODE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log() 142 143 if (ffa_id < FFA_FIRST_ID || ffa_id > FFA_LAST_ID) 144 return -EINVAL; 145 146 abi_idx = FFA_ID_TO_ERRMAP_ID(ffa_id); 147 if (abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)
CID 464361: Control flow issues (DEADCODE) Execution cannot reach this statement: "return -22;".
148 return -EINVAL; 149 150 if (!err_msg_map[abi_idx].err_str[err_idx]) 151 return -EINVAL; 152 153 log_err("%s\n", err_msg_map[abi_idx].err_str[err_idx]);
** CID 464360: Control flow issues (NO_EFFECT) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 207 in ffa_get_version_hdlr()
________________________________________________________________________________________________________ *** CID 464360: Control flow issues (NO_EFFECT) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 207 in ffa_get_version_hdlr() 201 major = GET_FFA_MAJOR_VERSION(res.a0); 202 minor = GET_FFA_MINOR_VERSION(res.a0); 203 204 log_debug("FF-A driver %d.%d\nFF-A framework %d.%d\n", 205 FFA_MAJOR_VERSION, FFA_MINOR_VERSION, major, minor); 206
CID 464360: Control flow issues (NO_EFFECT) This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "minor >= 0".
207 if (major == FFA_MAJOR_VERSION && minor >= FFA_MINOR_VERSION) { 208 log_debug("FF-A versions are compatible\n"); 209 210 if (dev) { 211 uc_priv = dev_get_uclass_priv(dev); 212 if (uc_priv)
** CID 464359: (PASS_BY_VALUE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 168 in invoke_ffa_fn() /drivers/firmware/arm-ffa/ffa-emul-uclass.c: 673 in invoke_ffa_fn()
________________________________________________________________________________________________________ *** CID 464359: (PASS_BY_VALUE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 168 in invoke_ffa_fn() 162 * @args: FF-A ABI arguments to be copied to Xn registers 163 * @res: FF-A ABI return data to be copied from Xn registers 164 * 165 * Calls low level SMC implementation. 166 * This function should be implemented by the user driver. 167 */
CID 464359: (PASS_BY_VALUE) Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.
168 void __weak invoke_ffa_fn(ffa_value_t args, ffa_value_t *res) 169 { 170 } 171 172 /** 173 * ffa_get_version_hdlr() - FFA_VERSION handler function /drivers/firmware/arm-ffa/ffa-emul-uclass.c: 673 in invoke_ffa_fn() 667 * invoke_ffa_fn() - SMC wrapper 668 * @args: FF-A ABI arguments to be copied to Xn registers 669 * @res: FF-A ABI return data to be copied from Xn registers 670 * 671 * Calls the emulated SMC call. 672 */
CID 464359: (PASS_BY_VALUE) Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.
673 void invoke_ffa_fn(ffa_value_t args, ffa_value_t *res) 674 { 675 sandbox_arm_ffa_smccc_smc(&args, res); 676 } 677 678 /**
Hi Tom,
Here's the latest report
---------- Forwarded message --------- From: scan-admin@coverity.com Date: Mon, Aug 21, 2023 at 4:30 PM Subject: New Defects reported by Coverity Scan for Das U-Boot To: tom.rini@gmail.com
Hi,
Please find the latest report on new defect(s) introduced to Das U-Boot found with Coverity Scan.
4 new defect(s) introduced to Das U-Boot found with Coverity Scan.
New defect(s) Reported-by: Coverity Scan Showing 4 of 4 defect(s)
** CID 464361: Control flow issues (DEADCODE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log()
Well received, I started working on that. I'll provide a fix after coming back fom holidays (mid September)
Cheers, Abdellatif
Hello,
El jue, 24 ago. 2023 06:27, Abdellatif El Khlifi < abdellatif.elkhlifi@arm.com> escribió:
Hi Tom,
Here's the latest report
---------- Forwarded message --------- From: scan-admin@coverity.com Date: Mon, Aug 21, 2023 at 4:30 PM Subject: New Defects reported by Coverity Scan for Das U-Boot To: tom.rini@gmail.com
Hi,
Please find the latest report on new defect(s) introduced to Das U-Boot found with Coverity Scan.
4 new defect(s) introduced to Das U-Boot found with Coverity Scan.
New defect(s) Reported-by: Coverity Scan Showing 4 of 4 defect(s)
** CID 464361: Control flow issues (DEADCODE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log()
Well received, I started working on that. I'll provide a fix after coming back fom holidays (mid September)
Cheers, Abdellatif
Is there something I could do to help with this?
On Mon, Aug 28, 2023 at 01:09:17PM -0300, Alvaro Fernando García wrote:
Hello,
El jue, 24 ago. 2023 06:27, Abdellatif El Khlifi < abdellatif.elkhlifi@arm.com> escribió:
Hi Tom,
Here's the latest report
---------- Forwarded message --------- From: scan-admin@coverity.com Date: Mon, Aug 21, 2023 at 4:30 PM Subject: New Defects reported by Coverity Scan for Das U-Boot To: tom.rini@gmail.com
Hi,
Please find the latest report on new defect(s) introduced to Das U-Boot found with Coverity Scan.
4 new defect(s) introduced to Das U-Boot found with Coverity Scan.
New defect(s) Reported-by: Coverity Scan Showing 4 of 4 defect(s)
** CID 464361: Control flow issues (DEADCODE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log()
Well received, I started working on that. I'll provide a fix after coming back fom holidays (mid September)
Cheers, Abdellatif
Is there something I could do to help with this?
Everyone is free to work on these issues, yes.
Hi Tom,
*** CID 464361: Control flow issues (DEADCODE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log() 142 143 if (ffa_id < FFA_FIRST_ID || ffa_id > FFA_LAST_ID) 144 return -EINVAL; 145 146 abi_idx = FFA_ID_TO_ERRMAP_ID(ffa_id); 147 if (abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)
CID 464361: Control flow issues (DEADCODE) Execution cannot reach this statement: "return -22;".148 return -EINVAL;
This is a false positive.
abi_idx value could end up matching this condition "(abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)".
This happens when ffa_id value is above the allowed bounds. Example: when ffa_id is 0x50 or 0x80
ffa_print_error_log(0x50, ...); /* exceeding lower bound */ ffa_print_error_log(0x80, ...); /* exceeding upper bound */
In these cases "return -EINVAL;" is executed.
... ________________________________________________________________________________________________________ *** CID 464360: Control flow issues (NO_EFFECT) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 207 in ffa_get_version_hdlr() 201 major = GET_FFA_MAJOR_VERSION(res.a0); 202 minor = GET_FFA_MINOR_VERSION(res.a0); 203 204 log_debug("FF-A driver %d.%d\nFF-A framework %d.%d\n", 205 FFA_MAJOR_VERSION, FFA_MINOR_VERSION, major, minor); 206
CID 464360: Control flow issues (NO_EFFECT) This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "minor >= 0".207 if (major == FFA_MAJOR_VERSION && minor >= FFA_MINOR_VERSION) {
Providing the facts that:
#define FFA_MINOR_VERSION (0) u16 minor;
Yes, currently this condition is always true: minor >= FFA_MINOR_VERSION
However, we might upgrade FFA_MINOR_VERSION in the future. If we remove the "minor >= FFA_MINOR_VERSION" , non compatible versions could pass which we don't want.
To keep this code scalable, I think it's better to keep this condition.
... ________________________________________________________________________________________________________ *** CID 464359: (PASS_BY_VALUE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 168 in invoke_ffa_fn() 162 * @args: FF-A ABI arguments to be copied to Xn registers 163 * @res: FF-A ABI return data to be copied from Xn registers 164 * 165 * Calls low level SMC implementation. 166 * This function should be implemented by the user driver. 167 */
CID 464359: (PASS_BY_VALUE) Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.168 void __weak invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
We are using invoke_ffa_fn with the same arguments as in linux. The aim is to use the same interfaces as in the Linux FF-A driver to make porting code easier.
In Linux, args is passed by value [1]. ffa_value_t is a structure with 18 "unsigned long" fields. So, the size is fixed.
[1]: invoke_ffa_fn arguments in the Linux FF-A driver
https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/dr... https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/dr... https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/co...
[2]: include/linux/arm-smccc.h
169 { 170 } 171 172 /** 173 * ffa_get_version_hdlr() - FFA_VERSION handler function /drivers/firmware/arm-ffa/ffa-emul-uclass.c: 673 in invoke_ffa_fn() 667 * invoke_ffa_fn() - SMC wrapper 668 * @args: FF-A ABI arguments to be copied to Xn registers 669 * @res: FF-A ABI return data to be copied from Xn registers 670 * 671 * Calls the emulated SMC call. 672 */
CID 464359: (PASS_BY_VALUE) Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.673 void invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
Same feedback as above.
Cheers, Abdellatif
address the CID 464361 Control flow issues [1]
[1]: https://lore.kernel.org/all/20230821210927.GL3953269@bill-the-cat/
Signed-off-by: Abdellatif El Khlifi abdellatif.elkhlifi@arm.com Cc: Tom Rini trini@konsulko.com --- drivers/firmware/arm-ffa/arm-ffa-uclass.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/firmware/arm-ffa/arm-ffa-uclass.c b/drivers/firmware/arm-ffa/arm-ffa-uclass.c index 8c17b19eaf..f1e91d151e 100644 --- a/drivers/firmware/arm-ffa/arm-ffa-uclass.c +++ b/drivers/firmware/arm-ffa/arm-ffa-uclass.c @@ -144,8 +144,6 @@ static int ffa_print_error_log(u32 ffa_id, int ffa_errno) return -EINVAL;
abi_idx = FFA_ID_TO_ERRMAP_ID(ffa_id); - if (abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT) - return -EINVAL;
if (!err_msg_map[abi_idx].err_str[err_idx]) return -EINVAL;
On Fri, Oct 20, 2023 at 02:15:33PM +0100, Abdellatif El Khlifi wrote:
address the CID 464361 Control flow issues [1]
Signed-off-by: Abdellatif El Khlifi abdellatif.elkhlifi@arm.com Cc: Tom Rini trini@konsulko.com
Applied to u-boot/master, thanks!
On Fri, Oct 20, 2023 at 12:57:47PM +0100, Abdellatif El Khlifi wrote:
Hi Tom,
*** CID 464361: Control flow issues (DEADCODE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log() 142 143 if (ffa_id < FFA_FIRST_ID || ffa_id > FFA_LAST_ID) 144 return -EINVAL; 145 146 abi_idx = FFA_ID_TO_ERRMAP_ID(ffa_id); 147 if (abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)
CID 464361: Control flow issues (DEADCODE) Execution cannot reach this statement: "return -22;".148 return -EINVAL;
This is a false positive.
abi_idx value could end up matching this condition "(abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)".
This happens when ffa_id value is above the allowed bounds. Example: when ffa_id is 0x50 or 0x80
ffa_print_error_log(0x50, ...); /* exceeding lower bound */ ffa_print_error_log(0x80, ...); /* exceeding upper bound */
In these cases "return -EINVAL;" is executed.
So those invalid values aren't caught by the previous check that ffa_id falls within FFA_FIRST_ID to FFA_LAST_ID ?
... ________________________________________________________________________________________________________ *** CID 464360: Control flow issues (NO_EFFECT) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 207 in ffa_get_version_hdlr() 201 major = GET_FFA_MAJOR_VERSION(res.a0); 202 minor = GET_FFA_MINOR_VERSION(res.a0); 203 204 log_debug("FF-A driver %d.%d\nFF-A framework %d.%d\n", 205 FFA_MAJOR_VERSION, FFA_MINOR_VERSION, major, minor); 206
CID 464360: Control flow issues (NO_EFFECT) This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "minor >= 0".207 if (major == FFA_MAJOR_VERSION && minor >= FFA_MINOR_VERSION) {
Providing the facts that:
#define FFA_MINOR_VERSION (0) u16 minor;
Yes, currently this condition is always true: minor >= FFA_MINOR_VERSION
However, we might upgrade FFA_MINOR_VERSION in the future. If we remove the "minor >= FFA_MINOR_VERSION" , non compatible versions could pass which we don't want.
To keep this code scalable, I think it's better to keep this condition.
OK, thanks this makes sense as an intentional change for future sanity checking.
*** CID 464359: (PASS_BY_VALUE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 168 in invoke_ffa_fn() 162 * @args: FF-A ABI arguments to be copied to Xn registers 163 * @res: FF-A ABI return data to be copied from Xn registers 164 * 165 * Calls low level SMC implementation. 166 * This function should be implemented by the user driver. 167 */
CID 464359: (PASS_BY_VALUE) Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.168 void __weak invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
We are using invoke_ffa_fn with the same arguments as in linux. The aim is to use the same interfaces as in the Linux FF-A driver to make porting code easier.
In Linux, args is passed by value [1]. ffa_value_t is a structure with 18 "unsigned long" fields. So, the size is fixed.
[1]: invoke_ffa_fn arguments in the Linux FF-A driver
https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/dr... https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/dr... https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/co...
So this is intentional, OK.
169 { 170 } 171 172 /** 173 * ffa_get_version_hdlr() - FFA_VERSION handler function /drivers/firmware/arm-ffa/ffa-emul-uclass.c: 673 in invoke_ffa_fn() 667 * invoke_ffa_fn() - SMC wrapper 668 * @args: FF-A ABI arguments to be copied to Xn registers 669 * @res: FF-A ABI return data to be copied from Xn registers 670 * 671 * Calls the emulated SMC call. 672 */
CID 464359: (PASS_BY_VALUE) Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.673 void invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
Same feedback as above.
Thanks. I'll update the last 3 CIDs shortly.
Hi Tom,
*** CID 464361: Control flow issues (DEADCODE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log() 142 143 if (ffa_id < FFA_FIRST_ID || ffa_id > FFA_LAST_ID) 144 return -EINVAL; 145 146 abi_idx = FFA_ID_TO_ERRMAP_ID(ffa_id); 147 if (abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)
CID 464361: Control flow issues (DEADCODE) Execution cannot reach this statement: "return -22;".148 return -EINVAL;
This is a false positive.
abi_idx value could end up matching this condition "(abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)".
This happens when ffa_id value is above the allowed bounds. Example: when ffa_id is 0x50 or 0x80
ffa_print_error_log(0x50, ...); /* exceeding lower bound */ ffa_print_error_log(0x80, ...); /* exceeding upper bound */
In these cases "return -EINVAL;" is executed.
So those invalid values aren't caught by the previous check that ffa_id falls within FFA_FIRST_ID to FFA_LAST_ID ?
I had a closer look at that and I agree that the deadcode defect is legitimate. I already provided a fix [1].
[1]: https://lore.kernel.org/all/20231020131533.239591-1-abdellatif.elkhlifi@arm....
... ________________________________________________________________________________________________________ *** CID 464360: Control flow issues (NO_EFFECT) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 207 in ffa_get_version_hdlr() 201 major = GET_FFA_MAJOR_VERSION(res.a0); 202 minor = GET_FFA_MINOR_VERSION(res.a0); 203 204 log_debug("FF-A driver %d.%d\nFF-A framework %d.%d\n", 205 FFA_MAJOR_VERSION, FFA_MINOR_VERSION, major, minor); 206
CID 464360: Control flow issues (NO_EFFECT) This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "minor >= 0".207 if (major == FFA_MAJOR_VERSION && minor >= FFA_MINOR_VERSION) {
Providing the facts that:
#define FFA_MINOR_VERSION (0) u16 minor;
Yes, currently this condition is always true: minor >= FFA_MINOR_VERSION
However, we might upgrade FFA_MINOR_VERSION in the future. If we remove the "minor >= FFA_MINOR_VERSION" , non compatible versions could pass which we don't want.
To keep this code scalable, I think it's better to keep this condition.
OK, thanks this makes sense as an intentional change for future sanity checking.
*** CID 464359: (PASS_BY_VALUE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 168 in invoke_ffa_fn() 162 * @args: FF-A ABI arguments to be copied to Xn registers 163 * @res: FF-A ABI return data to be copied from Xn registers 164 * 165 * Calls low level SMC implementation. 166 * This function should be implemented by the user driver. 167 */
CID 464359: (PASS_BY_VALUE) Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.168 void __weak invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
We are using invoke_ffa_fn with the same arguments as in linux. The aim is to use the same interfaces as in the Linux FF-A driver to make porting code easier.
In Linux, args is passed by value [1]. ffa_value_t is a structure with 18 "unsigned long" fields. So, the size is fixed.
[1]: invoke_ffa_fn arguments in the Linux FF-A driver
https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/dr... https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/dr... https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/co...
So this is intentional, OK.
169 { 170 } 171 172 /** 173 * ffa_get_version_hdlr() - FFA_VERSION handler function /drivers/firmware/arm-ffa/ffa-emul-uclass.c: 673 in invoke_ffa_fn() 667 * invoke_ffa_fn() - SMC wrapper 668 * @args: FF-A ABI arguments to be copied to Xn registers 669 * @res: FF-A ABI return data to be copied from Xn registers 670 * 671 * Calls the emulated SMC call. 672 */
CID 464359: (PASS_BY_VALUE) Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.673 void invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
Same feedback as above.
Thanks. I'll update the last 3 CIDs shortly.
Thanks Tom :)
Cheers, Abdellatif
On Wed, Oct 25, 2023 at 04:12:37PM +0100, Abdellatif El Khlifi wrote:
Hi Tom,
*** CID 464361: Control flow issues (DEADCODE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log() 142 143 if (ffa_id < FFA_FIRST_ID || ffa_id > FFA_LAST_ID) 144 return -EINVAL; 145 146 abi_idx = FFA_ID_TO_ERRMAP_ID(ffa_id); 147 if (abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)
> CID 464361: Control flow issues (DEADCODE) > Execution cannot reach this statement: "return -22;".
148 return -EINVAL;
This is a false positive.
abi_idx value could end up matching this condition "(abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)".
This happens when ffa_id value is above the allowed bounds. Example: when ffa_id is 0x50 or 0x80
ffa_print_error_log(0x50, ...); /* exceeding lower bound */ ffa_print_error_log(0x80, ...); /* exceeding upper bound */
In these cases "return -EINVAL;" is executed.
So those invalid values aren't caught by the previous check that ffa_id falls within FFA_FIRST_ID to FFA_LAST_ID ?
I had a closer look at that and I agree that the deadcode defect is legitimate. I already provided a fix [1].
Ah thanks. I had seen that posted but not put that together with this email and assumed it was addressing something you hadn't talked about here because you agreed with it being an issue. I will pick up the above patch soon then.
Hi Tom,
*** CID 464361: Control flow issues (DEADCODE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log() 142 143 if (ffa_id < FFA_FIRST_ID || ffa_id > FFA_LAST_ID) 144 return -EINVAL; 145 146 abi_idx = FFA_ID_TO_ERRMAP_ID(ffa_id); 147 if (abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)
>> CID 464361: Control flow issues (DEADCODE) >> Execution cannot reach this statement: "return -22;".
148 return -EINVAL;
This is a false positive.
abi_idx value could end up matching this condition "(abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)".
This happens when ffa_id value is above the allowed bounds. Example: when ffa_id is 0x50 or 0x80
ffa_print_error_log(0x50, ...); /* exceeding lower bound */ ffa_print_error_log(0x80, ...); /* exceeding upper bound */
In these cases "return -EINVAL;" is executed.
So those invalid values aren't caught by the previous check that ffa_id falls within FFA_FIRST_ID to FFA_LAST_ID ?
I had a closer look at that and I agree that the deadcode defect is legitimate. I already provided a fix [1].
Ah thanks. I had seen that posted but not put that together with this email and assumed it was addressing something you hadn't talked about here because you agreed with it being an issue. I will pick up the above patch soon then.
Thank you very much.
Cheers, Abdellatif
participants (3)
-
Abdellatif El Khlifi -
Alvaro Fernando García -
Tom Rini