Re: Verifier running out of memory on ieee1275/powerpc64
> On Mar 19, 2020, at 10:37 AM, Stefan Berger wrote: > > On 3/18/20 6:17 PM, Simon Hardy wrote: I was wondering whether it would not be possible to load the raw file into memory, pass it to the firmware for hashing (and logging) via the verifier, and if we do not trust that the firmware treated the file data as a read-only array, load the file again into the same array right after. This way we wouldn't need more memory. [*] However, I am not sure how it fits into the architecture with the verifiers or whether the TPM verifier would have to take on a special role (possibly with a flag) then. You didn't pick up on the idea of a bigger heap. Is there a problem with the heap size somehow? My machine has GBs of memory, so it really shouldn't be a problem to get memory. >>> I think that's the problem to solve, at least for this platform, since none >>> of the verifiers will work due to the memory exhaustion issue. >> Increasing the heap size may be the easiest way for you to solve this >> problem. I am not so familiar with the issues around doing this, but I >> have seen recent discussion on this list. > > I found the lsmmap command and it shows me some impressive memory ranges to > be known: > > base_addr = 0x4000, length = 0x17c000, available RAM > > base_addr = 0x1f, length = 0x1, available RAM > > base_addr = 0x21, length = 0x1, available RAM > > base_addr = 0x200, length = 0x7bbf, available RAM > > base_addr = 0x7feff, length = 0x1, available RAM > > base_addr = 0x800903dc, length = 0x7ff6fc24, available RAM > > There's more than needed memory here. It doesn't seem to be available for > grub_malloc, though. On SPARC, I thought the code was never written to claim memory in different regions. Does that same problem exist on PPC? I ran into a lot of memory corruption problems with ofdisk. I don’t know if this will solve your problem, but you could try this patch: https://lists.gnu.org/archive/html/grub-devel/2016-06/msg00035.html I couldn’t get any new ieee1275 code to work without it. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: are posix-shell-compliant continuation lines valid/supported, or not, in /etc/default/grub ?
On 3/19/20 8:17 PM, PGNet Dev wrote: > On 3/19/20 8:04 PM, Eli Schwartz wrote: >> On 3/19/20 10:02 PM, Dimitri John Ledkov wrote: >>> I hope you do understand that it >>> is intended for all of the Ubuntu installer & upgrade integrations to keep >>> valid configs compatible. For anyone similarly interested, it looks like Ubu 18 *LTS* will NOT be getting a fast fix for 'this' Posix support for a bit, if at all, as it remains a ... "highly unusual configuration". https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/1868138/comments/8 "So given that 8.13 is affected - which was released a year ago, and it's a highly unusual configuration, this is not in need of an urgent hotfix." ::facepalm:: Eli, your admonition to consider a more 'stable' option moves to the front of the line. Migrations to Grub + Arch & OpenSUSE spinning up today. Thx for the comments folks. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: disk/mdraid1x_linux.c:181:15: warning: array subscript ...
Hi,
Paul Menzel wrote:
> 181 | (char *) _roles[grub_le_to_cpu32 (sb.dev_number)]
>98 | grub_uint16_t dev_roles[]; /* Role in array, or 0x for a
> 127 | struct grub_raid_super_1x sb;
> [...]
> Normally, it should be fixed by using `grub_uint16_t[]` instead of
> `grub_uint16_t[0]`, but I haven’t found out where yet.
I rather propose to consider this untested and not properly styled
change:
--- grub-core/disk/mdraid1x_linux.c 2018-09-05 11:41:09.690721520 +0200
+++ grub-core/disk/mdraid1x_linux.c.ts 2020-03-20 13:57:21.159675792 +0100
@@ -178,8 +178,9 @@ grub_mdraid_detect (grub_disk_t disk,
return NULL;
if (grub_disk_read (disk, sector,
- (char *) _roles[grub_le_to_cpu32
(sb.dev_number)]
- - (char *) ,
+ ((char *) _roles - (char *) sb)
+ + grub_le_to_cpu32 (sb.dev_number) *
sizeof(grub_uint16_t),
sizeof (role), ))
return NULL;
Reasoning:
I see
grub_uint16_t dev_roles[0];
in
http://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/disk/mdraid1x_linux.c#n98
It's a gcc extension.
https://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html#Declaring-Arrays
"As a GNU extension, the number of elements can be as small as zero.
Zero-length arrays are useful as the last element of a structure which
is really a header for a variable-length object"
So isn't the problem rather about the allocation of the struct which
hosts .dev_roles ?
Currently there is in mdraid1x_linux.c:
struct grub_raid_super_1x
{
...
grub_uint16_t dev_roles[0]; /* Role in array, or 0x for a spare, or
0xfffe for faulty. */
};
...
static struct grub_diskfilter_vg *
grub_mdraid_detect (grub_disk_t disk, ...
...
struct grub_raid_super_1x sb;
The allocation as local variable does not appear to provide this extra
memory storage, which shall host the array members of dev_roles.
I fail to see how sb could get enlarged later. The stack neighbors of sb
do not look like they would provide their storage for an array.
Now why didn't this fail earlier ?
That's because the bad array index use is not for memory access but for
pointer arithmetics:
http://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/disk/mdraid1x_linux.c#n180
if (grub_disk_read (disk, sector,
(char *) _roles[grub_le_to_cpu32 (sb.dev_number)]
- (char *) ,
sizeof (role), ))
The code wants a number which shall be used as parameter grub_off_t offset
of grub_disk_read() (in grub-core/kern/disk.c).
I think that the following expression produces the same number without
virtual access to a virtual array member:
(char *) _roles - (char *) sb
+ grub_le_to_cpu32 (sb.dev_number) * sizeof(grub_uint16_t)
Have a nice day :)
Thomas
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
disk/mdraid1x_linux.c:181:15: warning: array subscript is outside array bounds of ‘grub_uint16_t[0]’ {aka ‘short unsigned int[0]’} [-Warray-bounds]
Dear GRUB folks,
Using Debian Sid/unstable with
gcc (Debian 10-20200312-2) 10.0.1 20200312 (experimental) [master
revision c56871dd15a:7ba6e7f0f21:daf2852b883762d921361462dad1f99320faca2a]
building GRUB fails with the error below due to treating warnings as errors.
gcc -DHAVE_CONFIG_H -I. -I.. -Wall -W -DGRUB_MACHINE_EFI=1 -DGRUB_MACHINE=X86_64_EFI
-m64 -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/10/include -I../include
-I../include -DGRUB_FILE=\"disk/mdraid1x_linux.c\" -I. -I. -I.. -I..
-I../include -I../include -I../grub-core/lib/libgcrypt-grub/src/
-D_FILE_OFFSET_BITS=64 -Os -m64 -Wall -W -Wshadow -Wpointer-arith -Wundef
-Wchar-subscripts -Wcomment -Wdeprecated-declarations -Wdisabled-optimization
-Wdiv-by-zero -Wfloat-equal -Wformat-extra-args -Wformat-security -Wformat-y2k -Wimplicit
-Wimplicit-function-declaration -Wimplicit-int -Wmain -Wmissing-braces
-Wmissing-format-attribute -Wmultichar -Wparentheses -Wreturn-type -Wsequence-point
-Wshadow -Wsign-compare -Wswitch -Wtrigraphs -Wunknown-pragmas -Wunused -Wunused-function
-Wunused-label -Wunused-parameter -Wunused-value -Wunused-variable -Wwrite-strings
-Wnested-externs -Wstrict-prototypes -g -Wredundant-decls -Wmissing-prototypes
-Wmissing-declarations -Wextra -Wattributes -Wendif-labels -Winit-self
-Wint-to-pointer-cast -Winvalid-pch -Wmissing-field-initializers -Wnonnull -Woverflow
-Wvla -Wpointer-to-int-cast -Wstrict-aliasing -Wvariadic-macros -Wvolatile-register-var
-Wpointer-sign -Wmissing-include-dirs -Wmissing-prototypes -Wmissing-declarations
-Wformat=2 -freg-struct-return -mno-mmx -mno-sse -mno-sse2 -mno-sse3 -mno-3dnow
-msoft-float -fno-dwarf2-cfi-asm -mno-stack-arg-probe -fno-asynchronous-unwind-tables
-fno-unwind-tables -fno-ident -mcmodel=large -mno-red-zone -fno-PIE -fno-pie
-fno-stack-protector -Wtrampolines -ffreestanding -MT
disk/mdraid1x_module-mdraid1x_linux.o -MD -MP -MF
disk/.deps-core/mdraid1x_module-mdraid1x_linux.Tpo -c -o
disk/mdraid1x_module-mdraid1x_linux.o `test -f 'disk/mdraid1x_linux.c' || echo
'./'`disk/mdraid1x_linux.c
disk/mdraid1x_linux.c: In function ‘grub_mdraid_detect’:
disk/mdraid1x_linux.c:181:15: warning: array subscript is outside
array bounds of ‘grub_uint16_t[0]’ {aka ‘short unsigned int[0]’} [-Warray-bounds]
181 | (char *) _roles[grub_le_to_cpu32 (sb.dev_number)]
| ^~~
disk/mdraid1x_linux.c:98:17: note: while referencing ‘dev_roles’
98 | grub_uint16_t dev_roles[]; /* Role in array, or 0x for a spare,
or 0xfffe for faulty. */
| ^
disk/mdraid1x_linux.c:127:33: note: defined here ‘sb’
127 | struct grub_raid_super_1x sb;
|
Code in question seems to be:
if (grub_disk_read (disk, sector,
(char *) _roles[grub_le_to_cpu32
(sb.dev_number)]
- (char *) ,
sizeof (role), ))
return NULL;
Normally, it should be fixed by using `grub_uint16_t[]` instead of
`grub_uint16_t[0]`, but I haven’t found out where yet.
Kind regards,
Paul
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
