| Message ID | 20250104191006.3807-1-tanyaagarwal25699@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | [V2] lockdown: Initialize array before use | expand |
On Jan 4, 2025 Tanya Agarwal <tanyaagarwal25699@gmail.com> wrote: > > The static code analysis tool "Coverity Scan" pointed the following > details out for further development considerations: > CID 1486102: Uninitialized scalar variable (UNINIT) > uninit_use_in_call: Using uninitialized value *temp when calling > strlen. > > Conclusion: > Initialize array before use in lockdown_read(). > > Fixes: 000d388ed3bb ("security: Add a static lockdown policy LSM") > Signed-off-by: Tanya Agarwal <tanyaagarwal25699@gmail.com> > --- > V2: add Fixes tag and reword commit description > > Coverity Link: > https://scan7.scan.coverity.com/#/project-view/52849/11354?selectedIssue=1486102 > > security/lockdown/lockdown.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c > index f2bdbd55aa2b..df2c8435a7a9 100644 > --- a/security/lockdown/lockdown.c > +++ b/security/lockdown/lockdown.c > @@ -96,7 +96,7 @@ static int __init lockdown_lsm_init(void) > static ssize_t lockdown_read(struct file *filp, char __user *buf, size_t count, > loff_t *ppos) > { > - char temp[80]; > + char temp[80] = {0}; > int i, offset = 0; Since @temp is a string, it reads a bit better if we initialize it with double quotes, e.g. 'char temp[80] = ""'. This is also a case where the static analysis is a bit misleading. The @temp variable is uninitialized only if the @lockdown_levels array is empty and seeing as the @lockdown_levels array is a constant array defined in the source, this isn't a real issue we need to worry about. However, if you wanted to fix this simply to quiet Coverity, I think that would be okay, especially since this isn't a hot code path. > for (i = 0; i < ARRAY_SIZE(lockdown_levels); i++) { > -- > 2.39.5 -- paul-moore.com
On Sun, Jan 5, 2025 at 9:45 AM Paul Moore <paul@paul-moore.com> wrote: > > On Jan 4, 2025 Tanya Agarwal <tanyaagarwal25699@gmail.com> wrote: > > > > - char temp[80]; > > + char temp[80] = {0}; > > int i, offset = 0; > > Since @temp is a string, it reads a bit better if we initialize it with > double quotes, e.g. 'char temp[80] = ""'. Hi Paul, Thanks for the review. I agree to change it to double quotes and will send it in a revision patch. > > This is also a case where the static analysis is a bit misleading. The > @temp variable is uninitialized only if the @lockdown_levels array is > empty and seeing as the @lockdown_levels array is a constant array > defined in the source, this isn't a real issue we need to worry about. > However, if you wanted to fix this simply to quiet Coverity, I think > that would be okay, especially since this isn't a hot code path. I understand that is likely a false positive. However, as you mentioned this is not a hot path, making initialization explicit could help silence the static analyzer without any real downside. Thanks, Tanya > > > for (i = 0; i < ARRAY_SIZE(lockdown_levels); i++) { > > -- > > 2.39.5 > > -- > paul-moore.com
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c index f2bdbd55aa2b..df2c8435a7a9 100644 --- a/security/lockdown/lockdown.c +++ b/security/lockdown/lockdown.c @@ -96,7 +96,7 @@ static int __init lockdown_lsm_init(void) static ssize_t lockdown_read(struct file *filp, char __user *buf, size_t count, loff_t *ppos) { - char temp[80]; + char temp[80] = {0}; int i, offset = 0; for (i = 0; i < ARRAY_SIZE(lockdown_levels); i++) {