lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 28 Feb 2022 05:18:51 -0800 From: Tom Rix <trix@...hat.com> To: William Breathitt Gray <vilhelm.gray@...il.com>, Jonathan Cameron <jic23@...nel.org> Cc: nathan@...nel.org, ndesaulniers@...gle.com, Jonathan.Cameron@...wei.com, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, llvm@...ts.linux.dev Subject: Re: [PATCH] counter: add defaults to switch-statements On 2/28/22 5:03 AM, William Breathitt Gray wrote: > On Sun, Feb 27, 2022 at 05:46:10PM +0000, Jonathan Cameron wrote: >> On Sun, 27 Feb 2022 08:17:46 -0800 >> trix@...hat.com wrote: >> >>> From: Tom Rix <trix@...hat.com> >>> >>> Clang static analysis reports this representative problem >>> counter-chrdev.c:482:3: warning: Undefined or garbage value >>> returned to caller >>> return ret; >>> ^~~~~~~~~~ >>> >>> counter_get_data() has a multilevel switches, some without >>> defaults, so ret is sometimes not set. >>> Add returning -EINVAL similar to other defaults. >>> >>> Fixes: b6c50affda59 ("counter: Add character device interface") >>> Signed-off-by: Tom Rix <trix@...hat.com> >> I'm fairly sure this one is warning supression rather than a fix as >> that type u8 being used in the switch is storing an enum which is >> set only by kernel code and all values of the enum are covered. >> >> However, that's not locally visible so to me the addition looks good, I'd >> just be tempted to drop the fixes tag as backporting this looks like noise >> to me. >> >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com> > Yes, this isn't really a bug because the only values available are those > from the enum, but I see how this is not immediately obvious at first. > If Tom has no objections, I'll pick this up and drop the fixes tag so > that we have it along with the other counter changes for the 5.18 merge. Sounds good. Thanks, Tom > Thanks, > > William Breathitt Gray > >>> --- >>> drivers/counter/counter-chrdev.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/drivers/counter/counter-chrdev.c b/drivers/counter/counter-chrdev.c >>> index b7c62f957a6a8..69d340be9c93f 100644 >>> --- a/drivers/counter/counter-chrdev.c >>> +++ b/drivers/counter/counter-chrdev.c >>> @@ -477,6 +477,8 @@ static int counter_get_data(struct counter_device *const counter, >>> case COUNTER_SCOPE_COUNT: >>> ret = comp->count_u8_read(counter, parent, &value_u8); >>> break; >>> + default: >>> + return -EINVAL; >>> } >>> *value = value_u8; >>> return ret; >>> @@ -496,6 +498,8 @@ static int counter_get_data(struct counter_device *const counter, >>> case COUNTER_SCOPE_COUNT: >>> ret = comp->count_u32_read(counter, parent, &value_u32); >>> break; >>> + default: >>> + return -EINVAL; >>> } >>> *value = value_u32; >>> return ret;
Powered by blists - more mailing lists