Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot

Abdellatif El Khlifi
2023-10-31T14:21:44Z

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