On Mon, May 19, 2025 at 09:03:15PM -0500, Andrew Hamilton wrote:
> Correct ntfs_test test failures around attempting to validate attribute
> run list values. The calculation was incorrect for the 'curr' variable.
> With previous calculation, some file systems would fail validation
> despite being well-formed and valid. This was caused by incrementing
> 'curr' by min_size which included both the (already accounted for)
> min_size as well as the size of the run list. Correct by making a new
> variable 'run_size' to denote the current run list size to increment
> both 'curr' and 'min_size' separately.
>
> Fixes: 067b6d225 (fs/ntfs: Implement attribute verification)
>
> Signed-off-by: Andrew Hamilton <adham...@gmail.com>
> ---
> grub-core/fs/ntfs.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
> index b3117bf92..3eb70111b 100644
> --- a/grub-core/fs/ntfs.c
> +++ b/grub-core/fs/ntfs.c
> @@ -83,6 +83,7 @@ validate_attribute (grub_uint8_t *attr, void *end)
> {
> grub_size_t attr_size = 0;
> grub_size_t min_size = 0;
> + grub_size_t run_size = 0;
> grub_size_t spare = (grub_uint8_t *) end - attr;
> /*
> * Just used as a temporary variable to try and deal with cases where
> someone
> @@ -174,8 +175,10 @@ validate_attribute (grub_uint8_t *attr, void *end)
> * These directly follow the header byte, so we use them to update
> * the minimum size.
> */
If you update the code below should not you update the comment above?
> - min_size += (attr[curr] & 0x7) + ((attr[curr] >> 4) & 0x7);
> - curr += min_size;
> + run_size = (attr[curr] & 0x7) + ((attr[curr] >> 4) & 0x7);
> + curr += run_size;
> + curr++;
This...
> + min_size += run_size;
> min_size++;
... and this look strange even if they are correct. So, to make things
clear I would extend comment above with description why additional
increments are needed.
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
