Sounds good, I’ll make the update and send out a v3.
Thanks
Andrew
On Tue, May 20, 2025 at 10:29 AM Daniel Kiper <daniel.ki...@oracle.com>
wrote:
> 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
