Re: [PATCH] Fix integer overflow at left shift expression
--- Original Message ---
On Friday, December 16th, 2022 at 5:45 PM, Glenn Washburn
wrote:
>
> On Sun, 04 Dec 2022 13:06:37 +
> Maxim Fomin ma...@fomin.one wrote:
>
> > From db82faafba5e7eccd9fd6c0b7314f7322c1aecbd Mon Sep 17 00:00:00 2001
> > From: Maxim Fomin ma...@fomin.one
> > Date: Sun, 4 Dec 2022 12:05:34 +
> > Subject: [PATCH] Fix integer overflow at left shift expression.
> >
> > In case of large partitions (>1TiB) left shift
> > with signed int GRUB_DISK_SECTOR_BITS macro may
> > cause integer overflow which results in wrong
> > partition size.
> > ---
> > grub-core/kern/fs.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/grub-core/kern/fs.c b/grub-core/kern/fs.c
> > index b9508296d..c196f2bf1 100644
> > --- a/grub-core/kern/fs.c
> > +++ b/grub-core/kern/fs.c
> > @@ -130,7 +130,7 @@ grub_fs_probe (grub_device_t device)
> > struct grub_fs_block
> > {
> > grub_disk_addr_t offset;
> > - unsigned long length;
> > + grub_disk_addr_t length;
> > };
> >
> > static grub_err_t
> > @@ -195,7 +195,7 @@ grub_fs_blocklist_open (grub_file_t file, const
> > char *name) goto fail;
> > }
> >
> > - file->size += (blocks[i].length << GRUB_DISK_SECTOR_BITS);
> > + file->size += (blocks[i].length << (grub_disk_addr_t)
> > GRUB_DISK_SECTOR_BITS);
> > p++;
>
>
> Is this change actually necessary? You're making sure that
> GRUB_DISK_SECTOR_BITS is treated as a 64-bit integer, but it would be
> crazy for it to even be more than an 8 bit integer. Is there some other
> desirable effect of this?
>
> > }
>
>
> Glenn
Yes, casting GRUB_DISK_SECTOR_BITS is necessary according to my debug
work on this issue. Initially I changed only the type of 'length' struct
member, but it was not enough to fix this bug. Casting is necessary not
only to fit GRUB_DISK_SECTOR_BITS to bigger size integer, but to change
its type from signed to unsigned type. It seems it is necessary because
of integer promotion rules of C.
Best regards,
Maxim Fomin
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 0/3] term/serial: Add PCI-serial device support
On Fri, 26 Aug 2022 13:01:42 +0200 pet...@infradead.org wrote: > Hi! > > A small collection of patches that adds PCI serial support. > > The first patch is by Glenn and adds GRUB_HAS_PCI -- it is included > so that it is a complete posting. > > The second patch is the actual PCI serial support, and the last patch > a cleanup requested by Glenn. > > Much thanks to Glenn for all the feedback so far. > I've had to occasion to play with AMT recently, so I've tested this patch series and can confirm that it works as advertised. I added a reviewed by for one of the patches in this series many months ago, and I'll now formally add it to the whole series. The first patch has already been accepted, but I don't see why that should cause an issue in reviewed and merging the other two. Or does it? Tested-by: Glenn Washburn Reviewed-by: Glenn Washburn Glenn ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 1/1] Fix integer overflow at left shift expression on i386-pc platform
From 5db28aa0cb98e906adc7cb735bfa1979ce32c228 Mon Sep 17 00:00:00 2001
From: Maxim Fomin
Date: Sat, 17 Dec 2022 18:11:34 +
Subject: [PATCH v2 1/1] Fix integer overflow at left shift expression on
i386-pc platform.
In case of large partitions (>1TiB) left shift
expression with unsigned 'length' object and
signed GRUB_DISK_SECTOR_BITS macro may cause
integer overflow making calculated partition
size less than true value. This issue is fixed
by increasing the size of 'length' integer type
and casting GRUB_DISK_SECTOR_BITS to unsigned
type prior to shift expression.
Signed-off-by: Maxim Fomin
---
grub-core/kern/fs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/grub-core/kern/fs.c b/grub-core/kern/fs.c
index b9508296d..c196f2bf1 100644
--- a/grub-core/kern/fs.c
+++ b/grub-core/kern/fs.c
@@ -130,7 +130,7 @@ grub_fs_probe (grub_device_t device)
struct grub_fs_block
{
grub_disk_addr_t offset;
- unsigned long length;
+ grub_disk_addr_t length;
};
static grub_err_t
@@ -195,7 +195,7 @@ grub_fs_blocklist_open (grub_file_t file, const char *name)
goto fail;
}
- file->size += (blocks[i].length << GRUB_DISK_SECTOR_BITS);
+ file->size += (blocks[i].length << (grub_disk_addr_t)
GRUB_DISK_SECTOR_BITS);
p++;
}
--
2.39.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
