Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The local variable `r` is returned uninitialized when the user writes a value evaluating to false. --
commit 22a99a886bc0e01618cd42c5f2b503de86fa5a10 Author: Coiby Xu <[email protected]> crash_dump: Improve readability of config_keys_restore_store This commit introduces user input validation to config_keys_restore_store before restoring dm-crypt keys. It ensures the restore function's return code is verified and improves overall code readability. > diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c > index 9ad50ac54866a..b8fe630462cfa 100644 > --- a/kernel/crash_dump_dm_crypt.c > +++ b/kernel/crash_dump_dm_crypt.c > @@ -371,13 +371,25 @@ static ssize_t config_keys_restore_show(struct > config_item *item, char *page) > static ssize_t config_keys_restore_store(struct config_item *item, > const char *page, size_t count) > { > - if (!restore) > - restore_dm_crypt_keys_to_thread_keyring(); > + bool val; > + int r; > > - if (kstrtobool(page, &restore)) > + if (kstrtobool(page, &val)) > return -EINVAL; > > - return count; > + if (val) { > + if (restore) { > + pr_warn("dm-crypt keys already restored!\n"); > + return count; > + } > + r = restore_dm_crypt_keys_to_thread_keyring(); > + if (!r) { > + restore = true; > + r = count; > + } > + } > + [Severity: High] If a user writes a value evaluating to false (e.g., '0' or 'n') to this configfs attribute, kstrtobool() will succeed and set val to false. This causes the if (val) block to be bypassed entirely, resulting in the uninitialized variable r being returned here. Could this leak uninitialized kernel stack memory to userspace or cause undefined behavior? > + return r; > } > > CONFIGFS_ATTR(config_keys_, restore); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
