| Message ID | 20250415-fix_fs-v4-1-5d575124a3ff@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v4] fs/fs_parse: Remove unused and problematic validate_constant_table() | expand |
On Tue 15-04-25 20:25:00, Zijun Hu wrote: > From: Zijun Hu <quic_zijuhu@quicinc.com> > > Remove validate_constant_table() since: > > - It has no caller. > > - It has below 3 bugs for good constant table array array[] which must > end with a empty entry, and take below invocation for explaination: > validate_constant_table(array, ARRAY_SIZE(array), ...) > > - Always return wrong value due to the last empty entry. > - Imprecise error message for missorted case. > - Potential NULL pointer dereference since the last pr_err() may use > @tbl[i].name NULL pointer to print the last empty entry's name. > > Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> Looks good! Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > Changes in v4: > - Rebase on vfs-6.16.misc branch to fix merge conflict. > - Link to v3: https://lore.kernel.org/r/20250415-fix_fs-v3-1-0c378cc5ce35@quicinc.com > > Changes in v3: > - Remove validate_constant_table() instead of fixing it > - Link to v2: https://lore.kernel.org/r/20250411-fix_fs-v2-2-5d3395c102e4@quicinc.com > > Changes in v2: > - Remove fixes tag for remaining patches > - Add more comment for the NULL pointer dereference issue > - Link to v1: https://lore.kernel.org/r/20250410-fix_fs-v1-3-7c14ccc8ebaa@quicinc.com > --- > Documentation/filesystems/mount_api.rst | 15 ---------- > fs/fs_parser.c | 49 --------------------------------- > include/linux/fs_parser.h | 5 ---- > 3 files changed, 69 deletions(-) > > diff --git a/Documentation/filesystems/mount_api.rst b/Documentation/filesystems/mount_api.rst > index 47dafbb7427e6a829989a815e4d034e48fdbe7a2..e149b89118c885c377a17b95adcdbcb594b34e00 100644 > --- a/Documentation/filesystems/mount_api.rst > +++ b/Documentation/filesystems/mount_api.rst > @@ -752,21 +752,6 @@ process the parameters it is given. > If a match is found, the corresponding value is returned. If a match > isn't found, the not_found value is returned instead. > > - * :: > - > - bool validate_constant_table(const struct constant_table *tbl, > - size_t tbl_size, > - int low, int high, int special); > - > - Validate a constant table. Checks that all the elements are appropriately > - ordered, that there are no duplicates and that the values are between low > - and high inclusive, though provision is made for one allowable special > - value outside of that range. If no special value is required, special > - should just be set to lie inside the low-to-high range. > - > - If all is good, true is returned. If the table is invalid, errors are > - logged to the kernel log buffer and false is returned. > - > * :: > > bool fs_validate_description(const char *name, > diff --git a/fs/fs_parser.c b/fs/fs_parser.c > index c5cb19788f74771a945801ceedeec69efed0e40a..c092a9f79e324bacbd950165a0eb66632cae9e03 100644 > --- a/fs/fs_parser.c > +++ b/fs/fs_parser.c > @@ -379,55 +379,6 @@ int fs_param_is_path(struct p_log *log, const struct fs_parameter_spec *p, > EXPORT_SYMBOL(fs_param_is_path); > > #ifdef CONFIG_VALIDATE_FS_PARSER > -/** > - * validate_constant_table - Validate a constant table > - * @tbl: The constant table to validate. > - * @tbl_size: The size of the table. > - * @low: The lowest permissible value. > - * @high: The highest permissible value. > - * @special: One special permissible value outside of the range. > - */ > -bool validate_constant_table(const struct constant_table *tbl, size_t tbl_size, > - int low, int high, int special) > -{ > - size_t i; > - bool good = true; > - > - if (tbl_size == 0) { > - pr_warn("VALIDATE C-TBL: Empty\n"); > - return true; > - } > - > - for (i = 0; i < tbl_size; i++) { > - if (!tbl[i].name) { > - pr_err("VALIDATE C-TBL[%zu]: Null\n", i); > - good = false; > - } else if (i > 0 && tbl[i - 1].name) { > - int c = strcmp(tbl[i-1].name, tbl[i].name); > - > - if (c == 0) { > - pr_err("VALIDATE C-TBL[%zu]: Duplicate %s\n", > - i, tbl[i].name); > - good = false; > - } > - if (c > 0) { > - pr_err("VALIDATE C-TBL[%zu]: Missorted %s>=%s\n", > - i, tbl[i-1].name, tbl[i].name); > - good = false; > - } > - } > - > - if (tbl[i].value != special && > - (tbl[i].value < low || tbl[i].value > high)) { > - pr_err("VALIDATE C-TBL[%zu]: %s->%d const out of range (%d-%d)\n", > - i, tbl[i].name, tbl[i].value, low, high); > - good = false; > - } > - } > - > - return good; > -} > - > /** > * fs_validate_description - Validate a parameter specification array > * @name: Owner name of the parameter specification array > diff --git a/include/linux/fs_parser.h b/include/linux/fs_parser.h > index 5057faf4f09182fa6e7ddd03fb17b066efd7e58b..5a0e897cae807bbf5472645735027883a6593e91 100644 > --- a/include/linux/fs_parser.h > +++ b/include/linux/fs_parser.h > @@ -87,14 +87,9 @@ extern int lookup_constant(const struct constant_table tbl[], const char *name, > extern const struct constant_table bool_names[]; > > #ifdef CONFIG_VALIDATE_FS_PARSER > -extern bool validate_constant_table(const struct constant_table *tbl, size_t tbl_size, > - int low, int high, int special); > extern bool fs_validate_description(const char *name, > const struct fs_parameter_spec *desc); > #else > -static inline bool validate_constant_table(const struct constant_table *tbl, size_t tbl_size, > - int low, int high, int special) > -{ return true; } > static inline bool fs_validate_description(const char *name, > const struct fs_parameter_spec *desc) > { return true; } > > --- > base-commit: 8cc42084abd926e3f005d7f5c23694c598b29cee > change-id: 20250410-fix_fs-6e0a97c4e59f > > Best regards, > -- > Zijun Hu <quic_zijuhu@quicinc.com> >
diff --git a/Documentation/filesystems/mount_api.rst b/Documentation/filesystems/mount_api.rst index 47dafbb7427e6a829989a815e4d034e48fdbe7a2..e149b89118c885c377a17b95adcdbcb594b34e00 100644 --- a/Documentation/filesystems/mount_api.rst +++ b/Documentation/filesystems/mount_api.rst @@ -752,21 +752,6 @@ process the parameters it is given. If a match is found, the corresponding value is returned. If a match isn't found, the not_found value is returned instead. - * :: - - bool validate_constant_table(const struct constant_table *tbl, - size_t tbl_size, - int low, int high, int special); - - Validate a constant table. Checks that all the elements are appropriately - ordered, that there are no duplicates and that the values are between low - and high inclusive, though provision is made for one allowable special - value outside of that range. If no special value is required, special - should just be set to lie inside the low-to-high range. - - If all is good, true is returned. If the table is invalid, errors are - logged to the kernel log buffer and false is returned. - * :: bool fs_validate_description(const char *name, diff --git a/fs/fs_parser.c b/fs/fs_parser.c index c5cb19788f74771a945801ceedeec69efed0e40a..c092a9f79e324bacbd950165a0eb66632cae9e03 100644 --- a/fs/fs_parser.c +++ b/fs/fs_parser.c @@ -379,55 +379,6 @@ int fs_param_is_path(struct p_log *log, const struct fs_parameter_spec *p, EXPORT_SYMBOL(fs_param_is_path); #ifdef CONFIG_VALIDATE_FS_PARSER -/** - * validate_constant_table - Validate a constant table - * @tbl: The constant table to validate. - * @tbl_size: The size of the table. - * @low: The lowest permissible value. - * @high: The highest permissible value. - * @special: One special permissible value outside of the range. - */ -bool validate_constant_table(const struct constant_table *tbl, size_t tbl_size, - int low, int high, int special) -{ - size_t i; - bool good = true; - - if (tbl_size == 0) { - pr_warn("VALIDATE C-TBL: Empty\n"); - return true; - } - - for (i = 0; i < tbl_size; i++) { - if (!tbl[i].name) { - pr_err("VALIDATE C-TBL[%zu]: Null\n", i); - good = false; - } else if (i > 0 && tbl[i - 1].name) { - int c = strcmp(tbl[i-1].name, tbl[i].name); - - if (c == 0) { - pr_err("VALIDATE C-TBL[%zu]: Duplicate %s\n", - i, tbl[i].name); - good = false; - } - if (c > 0) { - pr_err("VALIDATE C-TBL[%zu]: Missorted %s>=%s\n", - i, tbl[i-1].name, tbl[i].name); - good = false; - } - } - - if (tbl[i].value != special && - (tbl[i].value < low || tbl[i].value > high)) { - pr_err("VALIDATE C-TBL[%zu]: %s->%d const out of range (%d-%d)\n", - i, tbl[i].name, tbl[i].value, low, high); - good = false; - } - } - - return good; -} - /** * fs_validate_description - Validate a parameter specification array * @name: Owner name of the parameter specification array diff --git a/include/linux/fs_parser.h b/include/linux/fs_parser.h index 5057faf4f09182fa6e7ddd03fb17b066efd7e58b..5a0e897cae807bbf5472645735027883a6593e91 100644 --- a/include/linux/fs_parser.h +++ b/include/linux/fs_parser.h @@ -87,14 +87,9 @@ extern int lookup_constant(const struct constant_table tbl[], const char *name, extern const struct constant_table bool_names[]; #ifdef CONFIG_VALIDATE_FS_PARSER -extern bool validate_constant_table(const struct constant_table *tbl, size_t tbl_size, - int low, int high, int special); extern bool fs_validate_description(const char *name, const struct fs_parameter_spec *desc); #else -static inline bool validate_constant_table(const struct constant_table *tbl, size_t tbl_size, - int low, int high, int special) -{ return true; } static inline bool fs_validate_description(const char *name, const struct fs_parameter_spec *desc) { return true; }