Re: [PATCH] loader: Ensure the newc pathname is NULL-terminated
On Thu, Nov 24, 2022 at 11:54:28AM +0800, Gary Lin via Grub-devel wrote:
> On Wed, Nov 23, 2022 at 03:44:54PM +0100, Daniel Kiper wrote:
> > On Wed, Nov 23, 2022 at 02:40:21PM +0800, Gary Lin via Grub-devel wrote:
> > > Per "man 5 cpio", the namesize in the cpio header includes the trailing
> > > NULL byte of the pathname and the pathname is followed by NULL bytes, but
> >
> > s/NULL/NUL/ and below please...
> >
> Will fix in v2.
Thanks!
> > > the current implementation excludes the trailing NULL byte when making
> > > the newc header plus the pathname. Although make_header() would pad the
> > > pathname string, the padding won't happen when strlen(name) +
> > > sizeof(struct newc_head) is a multiple of 4, and the non-NULL-terminated
> > > pathname may lead to unexpected results.
> > >
> > > For example, when loading linux with the following command:
> > >
> > > linux /boot/vmlinuz
> > > initrd newc:test12:/boot/test12 /boot/initrd
> > >
> > > Since strlen("test12") + sizeof(struct newc_head) is 116 = 29 * 4, there
> > > is no padding for the pathname, and the following error may show during
> > > linux boot:
> > >
> > > Initramfs unpacking failed: ZSTD-compressed data is trunc
> > >
> > > To avoid the potential problems, this commit includes the NULL byte when
> > > calling make_header() and adjusts the initrd size accordingly.
> > >
> > > Signed-off-by: Gary Lin
> > > ---
> > > grub-core/loader/linux.c | 16 +---
> > > 1 file changed, 9 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/grub-core/loader/linux.c b/grub-core/loader/linux.c
> > > index 830360172..2b26d293f 100644
> > > --- a/grub-core/loader/linux.c
> > > +++ b/grub-core/loader/linux.c
> > > @@ -127,12 +127,14 @@ insert_dir (const char *name, struct dir **root,
> > > n->name = grub_strndup (cb, ce - cb);
> > > if (ptr)
> > > {
> > > + char *tmp_name = grub_strndup (name, ce - name);
> >
> > Why is this needed?
> Because I need the extra '\0' for make_header(). For example, if "name"
> is "dir123/file", insert_dir() only sends 'd''i''r''1''2''3' to
> make_header() to create the header like this:
>
> 0070 30 37 30 37 30 31 32 31 38 33 32 41 37 43 30 30 |07070121832A7C00|
> 0080 30 30 34 31 45 44 30 30 30 30 30 33 45 38 30 30 |0041ED03E800|
> 0090 30 30 30 30 36 34 30 30 30 30 30 30 30 33 36 33 |64000363|
> 00a0 37 37 32 44 43 38 30 30 30 30 30 30 30 30 30 30 |772DC800|
> 00b0 30 30 30 30 30 38 30 30 30 30 30 30 31 33 30 30 |08001300|
> 00c0 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 ||
> 00d0 30 30 30 30 30 36 30 30 30 30 30 30 30 30 64 69 |06di|
> 00e0 72 31 32 33 30 37 30 37 30 31 31 31 34 44 37 46 |r123070701114D7F|
>^
>end of "dir123"
>
> As you can see, the pathname is ended without '\0', and it violates the
> cpio format. If we give make_header() the extra '\0':
>
> 0070 30 37 30 37 30 31 32 31 38 33 32 41 37 43 30 30 |07070121832A7C00|
> 0080 30 30 34 31 45 44 30 30 30 30 30 33 45 38 30 30 |0041ED03E800|
> 0090 30 30 30 30 36 34 30 30 30 30 30 30 30 33 36 33 |64000363|
> 00a0 37 37 32 44 43 38 30 30 30 30 30 30 30 30 30 30 |772DC800|
> 00b0 30 30 30 30 30 38 30 30 30 30 30 30 31 33 30 30 |08001300|
> 00c0 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 ||
> 00d0 30 30 30 30 30 37 30 30 30 30 30 30 30 30 64 69 |07di|
> 00e0 72 31 32 33 00 00 00 00 30 37 30 37 30 31 31 31 |r12307070111|
>
> There are 3 more NUL bytes appended, and the user can safely read the
> pathname without a further check.
OK, this is what I expected. Though please add a comment before
grub_strndup() call and shortly say what is happening here.
> > And if it is needed you have to check for NULL here.
> Okay. Will add the check in v2.
>
> > I do not mention that formatting is broken too. You use spaces instead
> > of tabs here and below.
> >
> I thought the indentation is 2-space without tab. Will convert 8 spaces
> into one tab.
Cool! Thank you!
> > > grub_dprintf ("linux", "Creating directory %s, %s\n", name, ce);
> > > - ptr = make_header (ptr, name, ce - name,
> > > + ptr = make_header (ptr, tmp_name, ce - name + 1,
> > >040777, 0);
> > > + grub_free (tmp_name);
> > > }
> > > if (grub_add (*size,
> > > - ALIGN_UP ((ce - (char *) name)
> > > + ALIGN_UP ((ce - (char *) name + 1)
> > > + sizeof (struct newc_head), 4),
> > > size))
> > > {
> > > @@ -191,7 +193,7 @@ grub_initrd_init (int argc, char *argv[],
> > > grub_initrd_close (initrd_ctx);
> > > return grub_errno;
> > > }
> > > - name_len = grub_strlen (initrd_ctx->components[i].newc_name);
> > > + nam
Re: Possible memory fault in fs/iso9660 (correction)
On Sat, Nov 19, 2022 at 01:57:59PM +0100, Thomas Schmitt wrote: > Hi, > > i wrote: > > I think the loop end condition should use 4 rather than 1: > > (char *) entry < (char *) sua + sua_size - 4 && entry->len > 0 > > Urm ... better "3 rather than 1": > >(char *) entry < (char *) sua + sua_size - 3 && entry->len > 0 > > The memory fault by entry->len will appear if > entry >= sua + sua_size - 2 > > > (Only good i did not submit a patch attempt. > Why is that "- 1" present anyways ? Shall it ensure the presence of > entry->type ?) I am not an ISO format expert but your thinking LGTM. So, could you send a patch fixing this issue? Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [programmer11...@programist.ru: Bug#1021846: grub-install is broken since 2.06-3: error: unknown filesystem]
Adding Daniel Axtens... On Tue, Nov 15, 2022 at 06:31:45PM +, Steve McIntyre wrote: > Hi all! > > программист некто (in CC) reported this bug a few weeks back in > Debian. Since I applied the bundle of filesystem bounds-checking fixes > a few months back, he can't run grub-install. He's done the work to > determine that the patch that breaks things for him is > > 2d014248d540c7e087934a94b6e7a2aa7fc2c704 fs/f2fs: Do not read past the end of > nat journal entries > > The full thread of our discussion is at https://bugs.debian.org/1021846 > > I don't have any knowledge of f2fs to go any further here. Help please! :-) > > - Forwarded message from программист некто > - > > From: программист некто > To: sub...@bugs.debian.org > Date: Sat, 15 Oct 2022 23:54:36 +0300 > Subject: Bug#1021846: grub-install is broken since 2.06-3: error: unknown > filesystem > Message-Id: <3168731665867...@wf4nrjvtssjecb53.iva.yp-c.yandex.net> > > Package: grub-pc > Version: 2.06-3~deb11u1 > Severity: critical > > Hello. Since version 2.06-3, grub-install is broken: it fails with "error: > unknown filesystem". > I test command /usr/sbin/grub-install -v /dev/sda > in some versions. Results in mail attachments. > Versions older than 2.06-3 works without error (2.06-2 and lower). > Tested versions: 2.04-20, 2.06-1, 2.06-2, 2.06-3~deb10u1, 2.06-3~deb11u1, > 2.06-4. > > Disk partitions: > > # fdisk --list-details > Disk /dev/sda: 29,82 GiB, 32017047552 bytes, 62533296 sectors > Disk model: TS32GSSD370S > Units: sectors of 1 * 512 = 512 bytes > Sector size (logical/physical): 512 bytes / 512 bytes > I/O size (minimum/optimal): 512 bytes / 512 bytes > Disklabel type: dos > Disk identifier: 0xc7177f7e > > Device Boot Start End Sectors Id Type Start-C/H/S End-C/H/S Attrs > /dev/sda1 2048 22763519 22761472 83 Linux 4/4/1 1023/254/2 > /dev/sda2 * 25866240 62531583 36665344 7 HPFS/ 1023/254/2 1023/254/2 80 > > $ disktype /dev/sda1 > --- /dev/sda1 > Block device, size 10.85 GiB (11653873664 bytes) > F2FS file system (version 1.14) > > $ disktype /dev/sda2 > --- /dev/sda2 > Block device, size 17.48 GiB (18772656128 bytes) > NTFS file system > Volume size 17.48 GiB (18772652032 bytes, 36665336 sectors) > > - End forwarded message - > -- > Steve McIntyre, Cambridge, UK.st...@einval.com > Mature Sporty Personal > More Innovation More Adult > A Man in Dandism > Powered Midship Specialty ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: Possible memory fault in fs/iso9660 (correction)
Hi, (Again i Cc t.feng in the hope that the review is not finished yet. :)) Daniel Kiper wrote: > I am not an ISO format expert but your thinking LGTM. So you agree that "3" is really the right number if any remaining bytes fewer than 4 shall be ignored ? (I don't trust myself, although i made an example with finger counting.) > could you send a patch fixing this issue? This should be possible. But how to test ? Normal ISOs made with GNU/Linux will cause (entry == sua + sua_size) at the end of the SUSP data. So provoking the problem and checking whether it is solved will need a hacked ISO. I will think about creating such an ISO by help of xorriso and dd. While exploring the SUSP/RRIP entry types which are interpreted by GRUB, i got to more credulence towards the ISO producer. E.g. in grub-core/fs/iso9660.c line 404 ff. /* The 2nd data byte stored how many bytes are skipped every time to get to the SUA (System Usage Area). */ data->susp_skip = entry->data[2]; This is a memory fault if (sua_size < 7). I see no check between sua = grub_malloc (sua_size); and the read access to entry->data[2]. Another example: grub_iso9660_susp_iterate() calls its parameter hook() without checking that the alleged entry size is within the range of sua_size. The hook() function does not get sua_size to check on its own. In general: How mistrusting should GRUB be towards the bytes in the filesystem ? Have a nice day :) Thomas ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present
On Mon, Oct 17, 2022 at 01:19:08PM +0800, Michael Chang via Grub-devel wrote: > On Fri, Oct 14, 2022 at 11:40:01AM +0200, Daniel Kiper wrote: > > On Fri, Oct 07, 2022 at 01:37:10PM +0800, Michael Chang via Grub-devel > > wrote: > > > This helps to prevent out of memory error when reading large files via > > > disabling > > > tpm device as verifier has to read all content into memory in one chunk to > > > measure the hash and extend to tpm. > > > > How does this patch help when the TPM is present in the system? > > If the firmware menu offers option to disable TPM device, then this > patch can be useful to get around 'out of memory error' through > disabling TPM device from firmware in order to make tpm verifier won't > be in the way of reading huge files. > > This is essentially a compromised solution as long as tpm module can be > a built-in module in signed image and at the same time user may come > across the need to open huge files, for eg, loopback mount in grub for > the rescue image. In this case they could be opted in to disable tpm > device from firmware to proceed if they run into out of memory or other > (slow) reading issues. I think I would prefer something similar to this [1] patch. Of course if [1] is not enough... Daniel [1] http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=a4356538d03c5a5350790b6453b523fb9214c2e9 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v3 0/2] Fix installation issues on ppc64le
On Mon, Sep 05, 2022 at 02:39:45PM +0200, Ismael Luceno wrote: > If the nvram device is non-functional, e.g. because the nvram module isn't > loaded and it's file been removed from the filesystem, thus can't be > loaded, the installation will be attempted but the system will be left in > an unbootable state. > > The boot process shows: > > Welcome to GRUB! > > error: ../../grub-core/kern/dl.c:380:symbol `grub_disk_get_size' not > found. > Entering rescue mode... > grub rescue> > > Introduce a point of no return after embedding grub in the PReP > partition, and make sure /dev/nvram is functional or fail early. > > Ismael Luceno (2): > grub-install: Ensure a functional /dev/nvram > grub-install: Add point of no return for IEEE1275 on powerpc Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [RESEND PATCH] templates: Set defaults using var substitution
On Wed, Aug 24, 2022 at 03:36:07PM +0200, Ismael Luceno wrote: > Signed-off-by: Ismael Luceno Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2
Hi,
Adding Sudhakar and Glenn...
On Thu, Aug 11, 2022 at 02:40:58PM -0300, Diego Domingos wrote:
> Hello,
>
> This is an addition to the series sent from Daniel Axtens
> (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).
>
> Patch 'ieee1275: request memory with ibm,client-architecture-support'
> implements vectors 1-4 of client-architecture-support negotiation
> However, during some tests, we found this can be a problem if:
>
> - we have more than 64 CPUs
> - Hardware Management Console (HMC) is configured to minimum of CPUs >64 (for
> example, min of 200 CPUs)
> - Grub needs to request memory.
>
> If vector 5 is not implemented, Power Hypervisor will consider the default
> value for vector 5 and 64 will bet set as the maximum
> number of CPUs supported by the OS, causing the machine to fail to init.
> Today we support 256 CPUs (max) on Power, so we need to implement vector 5
> and set the MAX CPUs bits to this value.
>
> The patches 11-15 aren't merged to the grub tree yet, so I'm sending those
> patches again together with my patch to implement vector 5
> on top of them.
>
> The patches 11-15 contains the following:
>
> Daniel Axtens (4):
> ieee1275: request memory with ibm,client-architecture-support
> ieee1275: drop len -= 1 quirk in heap_init
> ieee1275: support runtime memory claiming
> [RFC] Add memtool module with memory allocation stress-test
>
> Stefan Berger (1):
> ibmvtpm: Add support for trusted boot using a vTPM 2.0
I went through all patches and cannot see major problems with them.
Though there are a lot of minor things which have to be fixed. Sadly due
to number of them I cannot simply ignore that.
Here is the list of the issues:
- functions calls/sizeof(): e.g. "grub_printf()" should be replaced with
"grub_printf ()",
add space before "(", in the code; though I am OK with the former in
comments and
commit messages,
- casts: e.g. "*(grub_uint32_t *)data" should be replaced with
"*(grub_uint32_t *) data",
add space between ")" and "data",
- s/__attribute__((packed))/GRUB_PACKED/
- if you use grub_err_t type please test for GRUB_ERR_NONE instead of !err or
err;
please do not use plain numbers, e.g. 0 to substitute GRUB_ERR_NONE,
- if you test pointers for NULL please test using NULL constant instead of
e.g. !ptr
- if you use a value often please define constant for it; good candidate for
such
change is at least 0x3000 in the patch #3; if constant definition is an
overkill
please comment what given numbers/strings mean or at least where they come
from,
- please do not use "//" for comments,
- I am OK with lines a bit longer than 80; so, please do not wrap
lines too early,
- year in the copyright should be 2022.
The GRUB coding style is described here [1] and you can find good
example of coding style in the grub-core/kern/efi/sb.c file.
Please take into account latest comments from Daniel A. and Glenn too.
If something is not clear please drop me a line.
Last but not least, sorry for huge delay. I hope I will be able to
review much faster next version of this patch set.
Daniel
[1] https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Coding-style
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [programmer11...@programist.ru: Bug#1021846: grub-install is broken since 2.06-3: error: unknown filesystem]
Hi Steve, It seems invalid Commit id which you reported. It should be 4bd9877f62166b7e369773ab92fe24a39f6515f8 did you applied below patch and tested? Could you please confirm that. fs/f2fs: Do not read past the end of nat journal entries https://git.savannah.gnu.org/cgit/grub.git/patch/?id=4bd9877f62166b7e369773ab92fe24a39f6515f8 thanks, Sudhakar Kuppusamy On 2022-11-24 20:39, Daniel Kiper wrote: Adding Daniel Axtens... On Tue, Nov 15, 2022 at 06:31:45PM +, Steve McIntyre wrote: Hi all! программист некто (in CC) reported this bug a few weeks back in Debian. Since I applied the bundle of filesystem bounds-checking fixes a few months back, he can't run grub-install. He's done the work to determine that the patch that breaks things for him is 2d014248d540c7e087934a94b6e7a2aa7fc2c704 fs/f2fs: Do not read past the end of nat journal entries The full thread of our discussion is at https://bugs.debian.org/1021846 I don't have any knowledge of f2fs to go any further here. Help please! :-) - Forwarded message from программист некто - From: программист некто To: sub...@bugs.debian.org Date: Sat, 15 Oct 2022 23:54:36 +0300 Subject: Bug#1021846: grub-install is broken since 2.06-3: error: unknown filesystem Message-Id: <3168731665867...@wf4nrjvtssjecb53.iva.yp-c.yandex.net> Package: grub-pc Version: 2.06-3~deb11u1 Severity: critical Hello. Since version 2.06-3, grub-install is broken: it fails with "error: unknown filesystem". I test command /usr/sbin/grub-install -v /dev/sda in some versions. Results in mail attachments. Versions older than 2.06-3 works without error (2.06-2 and lower). Tested versions: 2.04-20, 2.06-1, 2.06-2, 2.06-3~deb10u1, 2.06-3~deb11u1, 2.06-4. Disk partitions: # fdisk --list-details Disk /dev/sda: 29,82 GiB, 32017047552 bytes, 62533296 sectors Disk model: TS32GSSD370S Units: sectors of 1 * 512 = 512 bytes Sector size (logical/physical): 512 bytes / 512 bytes I/O size (minimum/optimal): 512 bytes / 512 bytes Disklabel type: dos Disk identifier: 0xc7177f7e Device Boot Start End Sectors Id Type Start-C/H/S End-C/H/S Attrs /dev/sda1 2048 22763519 22761472 83 Linux 4/4/1 1023/254/2 /dev/sda2 * 25866240 62531583 36665344 7 HPFS/ 1023/254/2 1023/254/2 80 $ disktype /dev/sda1 --- /dev/sda1 Block device, size 10.85 GiB (11653873664 bytes) F2FS file system (version 1.14) $ disktype /dev/sda2 --- /dev/sda2 Block device, size 17.48 GiB (18772656128 bytes) NTFS file system Volume size 17.48 GiB (18772652032 bytes, 36665336 sectors) - End forwarded message - -- Steve McIntyre, Cambridge, UK. st...@einval.com Mature Sporty Personal More Innovation More Adult A Man in Dandism Powered Midship Specialty ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present
On Thu, Nov 24, 2022 at 05:04:48PM +0100, Daniel Kiper wrote: > On Mon, Oct 17, 2022 at 01:19:08PM +0800, Michael Chang via Grub-devel wrote: > > On Fri, Oct 14, 2022 at 11:40:01AM +0200, Daniel Kiper wrote: > > > On Fri, Oct 07, 2022 at 01:37:10PM +0800, Michael Chang via Grub-devel > > > wrote: > > > > This helps to prevent out of memory error when reading large files via > > > > disabling > > > > tpm device as verifier has to read all content into memory in one chunk > > > > to > > > > measure the hash and extend to tpm. > > > > > > How does this patch help when the TPM is present in the system? > > > > If the firmware menu offers option to disable TPM device, then this > > patch can be useful to get around 'out of memory error' through > > disabling TPM device from firmware in order to make tpm verifier won't > > be in the way of reading huge files. > > > > This is essentially a compromised solution as long as tpm module can be > > a built-in module in signed image and at the same time user may come > > across the need to open huge files, for eg, loopback mount in grub for > > the rescue image. In this case they could be opted in to disable tpm > > device from firmware to proceed if they run into out of memory or other > > (slow) reading issues. > > I think I would prefer something similar to this [1] patch. Of course > if [1] is not enough... The tpm verifier attempts to set GRUB_VERIFY_FLAGS_SINGLE_CHUNK for all incoming files, which gets loaded into memory in its entirety as an duplicated copy to disk files. The overhead is too huge to some low profile hardwares with smaller memory or when the boot path has to cover very large files, hence the out of memory error. I think it inevitable to use GRUB_VERIFY_FLAGS_SINGLE_CHUNK as tpm measures and extends file intergrity. But we ought to avoid the overhead when TPM device is not present or disabled by the user. The patch [1] seems to deal with the tpm error which prevents a file from being opened, which is orthogonal to the memory allocation issue in the common verifier before tpm doing measurement. Thanks, Michael > > Daniel > > [1] > http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=a4356538d03c5a5350790b6453b523fb9214c2e9 > > ___ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2] loader: Ensure the newc pathname is NULL-terminated
Per "man 5 cpio", the namesize in the cpio header includes the trailing
NUL byte of the pathname and the pathname is followed by NUL bytes, but
the current implementation ignores the trailing NUL byte when making
the newc header. Although make_header() tries to pad the pathname string,
the padding won't happen when strlen(name) + sizeof(struct newc_head)
is a multiple of 4, and the non-NULL-terminated pathname may lead to
unexpected results.
Assume that a file is created with 'echo -n > /boot/test12' and
loaded by grub2:
linux /boot/vmlinuz
initrd newc:test12:/boot/test12 /boot/initrd
The initrd command eventually invoked grub_initrd_load() and sent
't''e''s''t''1''2' to make_header() to generate the header:
0070 30 37 30 37 30 31 33 30 31 43 41 30 44 45 30 30 |070701301CA0DE00|
0080 30 30 38 31 41 34 30 30 30 30 30 33 45 38 30 30 |0081A403E800|
0090 30 30 30 30 36 34 30 30 30 30 30 30 30 31 36 33 |64000163|
00a0 37 36 45 34 35 32 30 30 30 30 30 30 30 34 30 30 |76E452000400|
00b0 30 30 30 30 30 38 30 30 30 30 30 30 31 33 30 30 |08001300|
00c0 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 ||
00d0 30 30 30 30 30 36 30 30 30 30 30 30 30 30 74 65 |06te|
^namesize
00e0 73 74 31 32 61 61 61 61 30 37 30 37 30 31 30 30 |st1207070100|
^^ end of the pathname
Since strlen("test12") + sizeof(struct newc_head) is 116 = 29 * 4,
make_header() didn't pad the pathname, and the file content followed
"test12" immediately. This violates the cpio format and may trigger such
error during linux boot:
Initramfs unpacking failed: ZSTD-compressed data is trunc
To avoid the potential problems, this commit counts the trailing NUL byte
in when calling make_header() and adjusts the initrd size accordingly.
Now the header becomes
0070 30 37 30 37 30 31 33 30 31 43 41 30 44 45 30 30 |070701301CA0DE00|
0080 30 30 38 31 41 34 30 30 30 30 30 33 45 38 30 30 |0081A403E800|
0090 30 30 30 30 36 34 30 30 30 30 30 30 30 31 36 33 |64000163|
00a0 37 36 45 34 35 32 30 30 30 30 30 30 30 34 30 30 |76E452000400|
00b0 30 30 30 30 30 38 30 30 30 30 30 30 31 33 30 30 |08001300|
00c0 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 ||
00d0 30 30 30 30 30 37 30 30 30 30 30 30 30 30 74 65 |07te|
^namesize
00e0 73 74 31 32 00 00 00 00 61 61 61 61 30 37 30 37 |st120707|
^^ end of the pathname
Besides the trailing NUL byte, make_header() pads 3 more NUL bytes, and
the user can safely read the pathname without a further check.
To conform to the cpio format, the headers for "TRAILER!!!" are also
adjusted to include the trailing NUL byte, not ignore it.
Signed-off-by: Gary Lin
---
v2:
- Fix the indentation
- Add more details in the commit comment
- Comment grub_strndup() in insert_dir()
- Add the missing size adjustment in grub_initrd_load()
---
grub-core/loader/linux.c | 27 +++
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/grub-core/loader/linux.c b/grub-core/loader/linux.c
index 830360172..2b5c42f74 100644
--- a/grub-core/loader/linux.c
+++ b/grub-core/loader/linux.c
@@ -127,12 +127,23 @@ insert_dir (const char *name, struct dir **root,
n->name = grub_strndup (cb, ce - cb);
if (ptr)
{
+ /*
+ * Create the substring with the trailing NUL byte
+ * to be included in the cpio header
+ */
+ char *tmp_name = grub_strndup (name, ce - name);
+ if (!tmp_name) {
+ grub_free (n->name);
+ grub_free (n);
+ return grub_errno;
+ }
grub_dprintf ("linux", "Creating directory %s, %s\n", name, ce);
- ptr = make_header (ptr, name, ce - name,
+ ptr = make_header (ptr, tmp_name, ce - name + 1,
040777, 0);
+ grub_free (tmp_name);
}
if (grub_add (*size,
- ALIGN_UP ((ce - (char *) name)
+ ALIGN_UP ((ce - (char *) name + 1)
+ sizeof (struct newc_head), 4),
size))
{
@@ -191,7 +202,7 @@ grub_initrd_init (int argc, char *argv[],
grub_initrd_close (initrd_ctx);
return grub_errno;
}
- name_len = grub_strlen (initrd_ctx->components[i].newc_name);
+ name_len = grub_strlen (initrd_ctx->components[i].newc_name) + 1;
if (grub_add (initrd_ctx->size,
ALIGN_UP (sizeof (struct newc_head) + name_len, 4),
&initrd_c
