[PATCH v2] fs/ntfs: Check at->attr_cur after calling next_attribute()

[PATCH v2] fs/ntfs: Check at->attr_cur after calling next_attribute()

[Andrew Hamilton]

A regression was introduced recently as a part of the series of
filesystem related patches to address some CVEs found in GRUB.

This issue may cause either an infinite loop at startup when
accessing certain valid NTFS file systems, or may cause a crash
due to a NULL pointer deference on systems where "NULL" address
is invalid (such as may happen when calling grub-mount from
the operating system level).

Correct this issue by checking that at->attr_cur != NULL inside
find_attr.

Fixes: https://savannah.gnu.org/bugs/?66855

Co-authored-by: B Horn <b@horn.uk>
Co-authored-by: Andrew Hamilton <adhamilt@gmail.com>
Signed-off-by: Andrew Hamilton <adhamilt@gmail.com>
---
 grub-core/fs/ntfs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index 960833a34..a29e10401 100644
--- a/grub-core/fs/ntfs.c
+++ b/grub-core/fs/ntfs.c
@@ -387,7 +387,8 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
     }
   at->attr_cur = at->attr_nxt;
   mft_end = at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR);
-  while (at->attr_cur < mft_end && *at->attr_cur != 0xFF)
+  while (at->attr_cur != NULL && at->attr_cur < mft_end
+         && *at->attr_cur != 0xFF)
     {
       at->attr_nxt = next_attribute (at->attr_cur, at->end);
       if (*at->attr_cur == GRUB_NTFS_AT_ATTRIBUTE_LIST)
-- 
2.39.5


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] fs/ntfs: Check at->attr_cur after calling next_attribute()

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1.1: Type: text/plain, Size: 1652 bytes --]

Le ven. 21 mars 2025, 01:54, Andrew Hamilton <adhamilt@gmail.com> a écrit :

> A regression was introduced recently as a part of the series of
> filesystem related patches to address some CVEs found in GRUB.
>
> This issue may cause either an infinite loop at startup when
> accessing certain valid NTFS file systems, or may cause a crash
> due to a NULL pointer deference on systems where "NULL" address
> is invalid (such as may happen when calling grub-mount from
> the operating system level).
>
> Correct this issue by checking that at->attr_cur != NULL inside
> find_attr.
>
> Fixes: https://savannah.gnu.org/bugs/?66855
>
> Co-authored-by: B Horn <b@horn.uk>
> Co-authored-by: Andrew Hamilton <adhamilt@gmail.com>
> Signed-off-by: Andrew Hamilton <adhamilt@gmail.com>
> ---
>  grub-core/fs/ntfs.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
> index 960833a34..a29e10401 100644
> --- a/grub-core/fs/ntfs.c
> +++ b/grub-core/fs/ntfs.c
> @@ -387,7 +387,8 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t
> attr)
>      }
>    at->attr_cur = at->attr_nxt;
>    mft_end = at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR);
> -  while (at->attr_cur < mft_end && *at->attr_cur != 0xFF)
> +  while (at->attr_cur != NULL && at->attr_cur < mft_end
> +         && *at->attr_cur != 0xFF)
>
Why not while (at->attr_cur >= at->mft->buf && at->attr_cur < mft_end &&
... ?

>      {
>        at->attr_nxt = next_attribute (at->attr_cur, at->end);
>        if (*at->attr_cur == GRUB_NTFS_AT_ATTRIBUTE_LIST)
> --
> 2.39.5
>
>

[-- Attachment #1.2: Type: text/html, Size: 2714 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] fs/ntfs: Check at->attr_cur after calling next_attribute()

[Andrew Hamilton]

[-- Attachment #1.1: Type: text/plain, Size: 1901 bytes --]

Good feedback, I will update as suggested and send out a v3 shortly!

Thanks!
Andrew

On Thu, Mar 20, 2025 at 6:04 PM Vladimir 'phcoder' Serbinenko <
phcoder@gmail.com> wrote:

>
>
> Le ven. 21 mars 2025, 01:54, Andrew Hamilton <adhamilt@gmail.com> a
> écrit :
>
>> A regression was introduced recently as a part of the series of
>> filesystem related patches to address some CVEs found in GRUB.
>>
>> This issue may cause either an infinite loop at startup when
>> accessing certain valid NTFS file systems, or may cause a crash
>> due to a NULL pointer deference on systems where "NULL" address
>> is invalid (such as may happen when calling grub-mount from
>> the operating system level).
>>
>> Correct this issue by checking that at->attr_cur != NULL inside
>> find_attr.
>>
>> Fixes: https://savannah.gnu.org/bugs/?66855
>>
>> Co-authored-by: B Horn <b@horn.uk>
>> Co-authored-by: Andrew Hamilton <adhamilt@gmail.com>
>> Signed-off-by: Andrew Hamilton <adhamilt@gmail.com>
>> ---
>>  grub-core/fs/ntfs.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
>> index 960833a34..a29e10401 100644
>> --- a/grub-core/fs/ntfs.c
>> +++ b/grub-core/fs/ntfs.c
>> @@ -387,7 +387,8 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t
>> attr)
>>      }
>>    at->attr_cur = at->attr_nxt;
>>    mft_end = at->mft->buf + (at->mft->data->mft_size <<
>> GRUB_NTFS_BLK_SHR);
>> -  while (at->attr_cur < mft_end && *at->attr_cur != 0xFF)
>> +  while (at->attr_cur != NULL && at->attr_cur < mft_end
>> +         && *at->attr_cur != 0xFF)
>>
> Why not while (at->attr_cur >= at->mft->buf && at->attr_cur < mft_end &&
> ... ?
>
>>      {
>>        at->attr_nxt = next_attribute (at->attr_cur, at->end);
>>        if (*at->attr_cur == GRUB_NTFS_AT_ATTRIBUTE_LIST)
>> --
>> 2.39.5
>>
>>

[-- Attachment #1.2: Type: text/html, Size: 3297 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] fs/ntfs: Check at->attr_cur after calling next_attribute()

[Ross Philipson via Grub-devel]

On 3/20/25 3:54 PM, Andrew Hamilton wrote:
> A regression was introduced recently as a part of the series of
> filesystem related patches to address some CVEs found in GRUB.
> 
> This issue may cause either an infinite loop at startup when
> accessing certain valid NTFS file systems, or may cause a crash
> due to a NULL pointer deference on systems where "NULL" address
> is invalid (such as may happen when calling grub-mount from
> the operating system level).
> 
> Correct this issue by checking that at->attr_cur != NULL inside
> find_attr.
> 
> Fixes: https://urldefense.com/v3/__https://savannah.gnu.org/bugs/?66855__;!!ACWV5N9M2RV99hQ!MlJos-JMiQzMR6XYrdHvoK1BCnCXRxxGUcBEZNXYJRmSe_ADsCWVO0Yzdk8RkShkoHuN6MBOT4mvwvWNMvBT$

Reviewed-by: Ross Philipson <ross.philipson@oracle.com>

> 
> Co-authored-by: B Horn <b@horn.uk>
> Co-authored-by: Andrew Hamilton <adhamilt@gmail.com>
> Signed-off-by: Andrew Hamilton <adhamilt@gmail.com>
> ---
>   grub-core/fs/ntfs.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
> index 960833a34..a29e10401 100644
> --- a/grub-core/fs/ntfs.c
> +++ b/grub-core/fs/ntfs.c
> @@ -387,7 +387,8 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
>       }
>     at->attr_cur = at->attr_nxt;
>     mft_end = at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR);
> -  while (at->attr_cur < mft_end && *at->attr_cur != 0xFF)
> +  while (at->attr_cur != NULL && at->attr_cur < mft_end
> +         && *at->attr_cur != 0xFF)
>       {
>         at->attr_nxt = next_attribute (at->attr_cur, at->end);
>         if (*at->attr_cur == GRUB_NTFS_AT_ATTRIBUTE_LIST)


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel