[PATCH] Issue reading >4GB loopback image via grub exfat

[PATCH] Issue reading >4GB loopback image via grub exfat

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

We have a big image (~6GB) in an exFAT partition. We want to read the content
of the image using the loopback command in grub.

What we get is an error message:

grub> loopback loop0 (hd1,gpt1)/endless/endless.img
grub> ls (loop0,gpt3)/boot
error: attempt to read or write outside of disk `loop0'.

We traced this error back to the wrong type for the 'file_size' variable in the
'grub_fat_data' struct. When MODE_EXFAT is defined and grub is compiled for
i386-pc, 'file_size' is still a 32bit variable when it should really be a 64bit
variable to fully contain the size of the >4GB file we are dealing with.

Carlo Caione (1):
  FAT/exFAT: Fix wrong size detection for files > 4GB

 grub-core/fs/fat.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH] FAT/exFAT: Fix wrong size detection for files > 4GB

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

For exFAT the file size can be > 4GB. The 'grub_fat_data' is still
marking the 'file_size' as grub_ssize_t that is always 32bit when
compiling for i386-pc also when using exFAT.

This is causing a problem when trying to open in loopback a > 4GB file
from an exFAT partition.

Fix this modifying the size of the 'file_size' according to the
MODE_EXFAT define.

Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
 grub-core/fs/fat.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/grub-core/fs/fat.c b/grub-core/fs/fat.c
index 79fe864..d5212c2 100644
--- a/grub-core/fs/fat.c
+++ b/grub-core/fs/fat.c
@@ -183,7 +183,11 @@ struct grub_fat_data
   grub_uint32_t num_clusters;
 
   grub_uint8_t attr;
-  grub_ssize_t file_size;
+#ifndef MODE_EXFAT
+  grub_uint32_t file_size;
+#else
+  grub_uint64_t file_size;
+#endif
   grub_uint32_t file_cluster;
   grub_uint32_t cur_cluster_num;
   grub_uint32_t cur_cluster;
-- 
2.7.4

Re: [PATCH] Issue reading >4GB loopback image via grub exfat

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 1157 bytes --]

You forgot to attach the patch

Le mer. 24 août 2016 17:52, Carlo Caione <carlo@caione.org> a écrit :

> From: Carlo Caione <carlo@endlessm.com>
>
> We have a big image (~6GB) in an exFAT partition. We want to read the
> content
> of the image using the loopback command in grub.
>
> What we get is an error message:
>
> grub> loopback loop0 (hd1,gpt1)/endless/endless.img
> grub> ls (loop0,gpt3)/boot
> error: attempt to read or write outside of disk `loop0'.
>
> We traced this error back to the wrong type for the 'file_size' variable
> in the
> 'grub_fat_data' struct. When MODE_EXFAT is defined and grub is compiled for
> i386-pc, 'file_size' is still a 32bit variable when it should really be a
> 64bit
> variable to fully contain the size of the >4GB file we are dealing with.
>
> Carlo Caione (1):
>   FAT/exFAT: Fix wrong size detection for files > 4GB
>
>  grub-core/fs/fat.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> --
> 2.7.4
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 1705 bytes --]

Re: [PATCH] Issue reading >4GB loopback image via grub exfat

[Carlo Caione]

On Wed, Aug 24, 2016 at 5:53 PM, Vladimir 'phcoder' Serbinenko
<phcoder@gmail.com> wrote:
> You forgot to attach the patch

I didn't. This is the cover letter, the patch is '[PATCH] FAT/exFAT:
Fix wrong size detection for files > 4GB'

Cheers,

-- 
Carlo Caione  |  +39.340.80.30.096  |  Endless

Re: [PATCH] FAT/exFAT: Fix wrong size detection for files > 4GB

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 1422 bytes --]

This makes it unsigned from signed. Did you check that no code relies on
signedness in fat.c?

Le mer. 24 août 2016 17:53, Carlo Caione <carlo@caione.org> a écrit :

> From: Carlo Caione <carlo@endlessm.com>
>
> For exFAT the file size can be > 4GB. The 'grub_fat_data' is still
> marking the 'file_size' as grub_ssize_t that is always 32bit when
> compiling for i386-pc also when using exFAT.
>
> This is causing a problem when trying to open in loopback a > 4GB file
> from an exFAT partition.
>
> Fix this modifying the size of the 'file_size' according to the
> MODE_EXFAT define.
>
> Signed-off-by: Carlo Caione <carlo@endlessm.com>
> ---
>  grub-core/fs/fat.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/fs/fat.c b/grub-core/fs/fat.c
> index 79fe864..d5212c2 100644
> --- a/grub-core/fs/fat.c
> +++ b/grub-core/fs/fat.c
> @@ -183,7 +183,11 @@ struct grub_fat_data
>    grub_uint32_t num_clusters;
>
>    grub_uint8_t attr;
> -  grub_ssize_t file_size;
> +#ifndef MODE_EXFAT
> +  grub_uint32_t file_size;
> +#else
> +  grub_uint64_t file_size;
> +#endif
>    grub_uint32_t file_cluster;
>    grub_uint32_t cur_cluster_num;
>    grub_uint32_t cur_cluster;
> --
> 2.7.4
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 2065 bytes --]

Re: [PATCH] FAT/exFAT: Fix wrong size detection for files > 4GB

[Carlo Caione]

On Wed, Aug 24, 2016 at 6:08 PM, Vladimir 'phcoder' Serbinenko
<phcoder@gmail.com> wrote:
> This makes it unsigned from signed. Did you check that no code relies on
> signedness in fat.c?

AFAICT there is no use for the signedness in fat.c

Also, if it were the case this code would be broken in fat.c

data->file_size = ctxt.dir.file_size

since ctxt.dir.file_size is grub_uint64_t for MODE_EXFAT and
grub_uint32_t for !MODE_EXFAT


-- 
Carlo Caione  |  +39.340.80.30.096  |  Endless

Re: [PATCH] FAT/exFAT: Fix wrong size detection for files > 4GB

[Andrei Borzenkov]

24.08.2016 16:36, Carlo Caione пишет:
> From: Carlo Caione <carlo@endlessm.com>
> 
> For exFAT the file size can be > 4GB. The 'grub_fat_data' is still
> marking the 'file_size' as grub_ssize_t that is always 32bit when
> compiling for i386-pc also when using exFAT.
> 
> This is causing a problem when trying to open in loopback a > 4GB file
> from an exFAT partition.
> 
> Fix this modifying the size of the 'file_size' according to the
> MODE_EXFAT define.
> 
> Signed-off-by: Carlo Caione <carlo@endlessm.com>
> ---
>  grub-core/fs/fat.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/grub-core/fs/fat.c b/grub-core/fs/fat.c
> index 79fe864..d5212c2 100644
> --- a/grub-core/fs/fat.c
> +++ b/grub-core/fs/fat.c
> @@ -183,7 +183,11 @@ struct grub_fat_data
>    grub_uint32_t num_clusters;
>  
>    grub_uint8_t attr;
> -  grub_ssize_t file_size;
> +#ifndef MODE_EXFAT
> +  grub_uint32_t file_size;
> +#else
> +  grub_uint64_t file_size;
> +#endif
>    grub_uint32_t file_cluster;
>    grub_uint32_t cur_cluster_num;
>    grub_uint32_t cur_cluster;
> 

Looks correct except I'd use conditon "ifdef MODE_EXFAT", negative
condition looks strange in such cases.

Re: [PATCH] FAT/exFAT: Fix wrong size detection for files > 4GB

[Carlo Caione]

On Wed, Aug 24, 2016 at 9:37 PM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> 24.08.2016 16:36, Carlo Caione пишет:
>> From: Carlo Caione <carlo@endlessm.com>
>>
>> For exFAT the file size can be > 4GB. The 'grub_fat_data' is still
>> marking the 'file_size' as grub_ssize_t that is always 32bit when
>> compiling for i386-pc also when using exFAT.
>>
>> This is causing a problem when trying to open in loopback a > 4GB file
>> from an exFAT partition.
>>
>> Fix this modifying the size of the 'file_size' according to the
>> MODE_EXFAT define.
>>
>> Signed-off-by: Carlo Caione <carlo@endlessm.com>
>> ---
>>  grub-core/fs/fat.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/grub-core/fs/fat.c b/grub-core/fs/fat.c
>> index 79fe864..d5212c2 100644
>> --- a/grub-core/fs/fat.c
>> +++ b/grub-core/fs/fat.c
>> @@ -183,7 +183,11 @@ struct grub_fat_data
>>    grub_uint32_t num_clusters;
>>
>>    grub_uint8_t attr;
>> -  grub_ssize_t file_size;
>> +#ifndef MODE_EXFAT
>> +  grub_uint32_t file_size;
>> +#else
>> +  grub_uint64_t file_size;
>> +#endif
>>    grub_uint32_t file_cluster;
>>    grub_uint32_t cur_cluster_num;
>>    grub_uint32_t cur_cluster;
>>
>
> Looks correct except I'd use conditon "ifdef MODE_EXFAT", negative
> condition looks strange in such cases.

This pattern is literally used 5 lines above and all over the place in fat.c
It would look strange otherwise.

-- 
Carlo Caione  |  +39.340.80.30.096  |  Endless

Re: [PATCH] FAT/exFAT: Fix wrong size detection for files > 4GB

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 1883 bytes --]

Committed, thanks. Sorry for delay

On Thu, 25 Aug 2016, 13:52 Carlo Caione <carlo@endlessm.com> wrote:

> On Wed, Aug 24, 2016 at 9:37 PM, Andrei Borzenkov <arvidjaar@gmail.com>
> wrote:
> > 24.08.2016 16:36, Carlo Caione пишет:
> >> From: Carlo Caione <carlo@endlessm.com>
> >>
> >> For exFAT the file size can be > 4GB. The 'grub_fat_data' is still
> >> marking the 'file_size' as grub_ssize_t that is always 32bit when
> >> compiling for i386-pc also when using exFAT.
> >>
> >> This is causing a problem when trying to open in loopback a > 4GB file
> >> from an exFAT partition.
> >>
> >> Fix this modifying the size of the 'file_size' according to the
> >> MODE_EXFAT define.
> >>
> >> Signed-off-by: Carlo Caione <carlo@endlessm.com>
> >> ---
> >>  grub-core/fs/fat.c | 6 +++++-
> >>  1 file changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/grub-core/fs/fat.c b/grub-core/fs/fat.c
> >> index 79fe864..d5212c2 100644
> >> --- a/grub-core/fs/fat.c
> >> +++ b/grub-core/fs/fat.c
> >> @@ -183,7 +183,11 @@ struct grub_fat_data
> >>    grub_uint32_t num_clusters;
> >>
> >>    grub_uint8_t attr;
> >> -  grub_ssize_t file_size;
> >> +#ifndef MODE_EXFAT
> >> +  grub_uint32_t file_size;
> >> +#else
> >> +  grub_uint64_t file_size;
> >> +#endif
> >>    grub_uint32_t file_cluster;
> >>    grub_uint32_t cur_cluster_num;
> >>    grub_uint32_t cur_cluster;
> >>
> >
> > Looks correct except I'd use conditon "ifdef MODE_EXFAT", negative
> > condition looks strange in such cases.
>
> This pattern is literally used 5 lines above and all over the place in
> fat.c
> It would look strange otherwise.
>
> --
> Carlo Caione  |  +39.340.80.30.096  |  Endless
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 3831 bytes --]