[PATCH v2] ofnet: Initialize structs in bootpath parser
Code later on checks if variables inside the struct are 0 to see if they have been set, like if there were addresses in the bootpath. The variables were not initialized however, so the check might succeed with uninitialized data, and a new interface with random addresses and the same name is added. This causes $net_default_mac to point to the random one, so, for example, using that variable to load per-mac config files fails. Bug-Ubuntu: https://bugs.launchpad.net/bugs/1785859 Signed-off-by: Julian Andres Klode --- Notes: v2: Rewrote commit message to address review comments and make it look nicer grub-core/net/drivers/ieee1275/ofnet.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/grub-core/net/drivers/ieee1275/ofnet.c b/grub-core/net/drivers/ieee1275/ofnet.c index 002446be1..00abc64bb 100644 --- a/grub-core/net/drivers/ieee1275/ofnet.c +++ b/grub-core/net/drivers/ieee1275/ofnet.c @@ -153,8 +153,8 @@ grub_ieee1275_parse_bootpath (const char *devpath, char *bootpath, char *comma_char = 0; char *equal_char = 0; grub_size_t field_counter = 0; - grub_net_network_level_address_t client_addr, gateway_addr, subnet_mask; - grub_net_link_level_address_t hw_addr; + grub_net_network_level_address_t client_addr = {}, gateway_addr = {}, subnet_mask = {}; + grub_net_link_level_address_t hw_addr = {}; grub_net_interface_flags_t flags = 0; struct grub_net_network_level_interface *inter = NULL; grub_uint16_t vlantag = 0; -- 2.17.1 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 7/9] btrfs: Add support for recovery for a RAID 5 btrfs profiles.
On Mon, Jun 18, 2018 at 08:12:22PM +0200, Goffredo Baroncelli wrote: > On 06/14/2018 09:05 PM, Goffredo Baroncelli wrote: > >>> + > >>> +cleanup: > >> Space before the label please. > >> I have asked about earlier. > > The line before the label is already a space; Am I missing something > > I think that now I understand: are you requiring a space before the > label *on the same line* ? Yep! > I found this style in some grub code, but this is not used everywhere > (ntfs.c and nilfs dont have the space, but xfs has it ). I know but I prefer label with space before it. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.
On Wed, Jul 18, 2018 at 08:24:50AM +0200, Goffredo Baroncelli wrote: > On 07/12/2018 03:46 PM, Daniel Kiper wrote: > > On Tue, Jun 19, 2018 at 07:39:48PM +0200, Goffredo Baroncelli wrote: > >> Signed-off-by: Goffredo Baroncelli > >> --- > >> grub-core/fs/btrfs.c | 70 > >> 1 file changed, 70 insertions(+) > >> > >> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c > >> index be195448d..fbabaebbe 100644 > >> --- a/grub-core/fs/btrfs.c > >> +++ b/grub-core/fs/btrfs.c > >> @@ -119,6 +119,8 @@ struct grub_btrfs_chunk_item > >> #define GRUB_BTRFS_CHUNK_TYPE_RAID1 0x10 > >> #define GRUB_BTRFS_CHUNK_TYPE_DUPLICATED0x20 > >> #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40 > >> +#define GRUB_BTRFS_CHUNK_TYPE_RAID5 0x80 > >> +#define GRUB_BTRFS_CHUNK_TYPE_RAID6 0x100 > >>grub_uint8_t dummy2[0xc]; > >>grub_uint16_t nstripes; > >>grub_uint16_t nsubstripes; > >> @@ -764,6 +766,74 @@ grub_btrfs_read_logical (struct grub_btrfs_data > >> *data, grub_disk_addr_t addr, > >> stripe_offset = low + chunk_stripe_length > >>* high; > >> csize = chunk_stripe_length - low; > >> +break; > >> + } > >> +case GRUB_BTRFS_CHUNK_TYPE_RAID5: > >> +case GRUB_BTRFS_CHUNK_TYPE_RAID6: > >> + { > >> +grub_uint64_t nparities, stripe_nr, high, low; > >> + > >> +redundancy = 1; /* no redundancy for now */ > >> + > >> +if (grub_le_to_cpu64 (chunk->type) & GRUB_BTRFS_CHUNK_TYPE_RAID5) > >> + { > >> +grub_dprintf ("btrfs", "RAID5\n"); > >> +nparities = 1; > >> + } > >> +else > >> + { > >> +grub_dprintf ("btrfs", "RAID6\n"); > >> +nparities = 2; > >> + } > >> + > >> +/* > >> + * Below is an example of a RAID 6 layout and the meaning of the > >> + * variables. The same applies to RAID 5. The only differences is > >> + * that there is only one parity disk instead of two. > >> + * > >> + * A RAID 6 layout consists of several stripes spread > >> + * on the disks, following a layout like the one below > >> + * > >> + * Disk0 Disk1 Disk2 Ddisk3 > > > > s/Ddisk3/Disk3/ > > > >> + * > >> + *A1 B1 P1 Q1 > >> + *Q2 A2 B2 P2 > >> + *P3 Q3 A3 B3 > >> + * [...] > >> + * > >> + * Note that the placement of the parities depends on row index. > >> + * In the code below: > >> + * - stripe_nr is the stripe number not considering the parities > >> + *(A1 = 0, B1 = 1, A2 = 2, B2 = 3, ...), > >> + * - high is the row number (0 for A1...Q1, 1 for Q2...P2, ...), > >> + * - stripen is the column number (or disk number), > >> + * - off is the logical address to read (from the beginning of > >> + *the chunk space), > > > > What do you mean by chunk? > > Is a specific btrfs data structure (see > https://btrfs.wiki.kernel.org/index.php/Manpage/mkfs.btrfs#BLOCK_GROUPS.2C_CHUNKS.2C_RAID). > Most of the BTRFS internal structures (e.g. where the file data is > stored) are in terms of *logical address*. At this level there is no > concept of redundancy or raid nor disks. You can see the logical > address as a flat space, large as the sum of the disks sizes (minus > the space used by the parities or mirroring). > > The "logical space" is divided in "slices" called chunk. A chunk > typically has a size in terms of hundred of megabytes (IIRC up to 1GB) > > A chunk (or block group) is a mapping. A chunk maps a "logical > address" to a "physical address" on the disks. > > You can think a chunk as a structure like: > > u64 profile > u64 logical_start, logical_end > u64 disk1_id, disk1_start, disk1_end > u64 disk2_id, disk2_start, disk2_end > u64 disk3_id, disk3_start, disk3_end > [...] > > In order to translate a "logical_address" to the pair of "disk" and "physical > address: > - find the chunk which maps the logical address (looking to the > "logical_start", "logical_end") > - then on the basis of "profile" value you can have the following cases: > profile == SINGLE: > is the simplest one: the logical address is mapped to the range > (disk1_start...disk1_end) of the disk "disk_id1" > > profile == RAID1 > like the previous one; however if the disk1 is not available, > you can find the data in the range > (disk2_start...disk2_end) of the disk "disk_id2". NOTE: the two > ranges > must have the same sizes, but may have different starts > > profile == RAID0 > the data is without redundancy and it is spread on different > disk each > "chunk_stripe_length" bytes; so: > off = logical_address
Re: [PATCH] grub-module-verifier: report the filename or modname in errors.
On Wed, Aug 01, 2018 at 12:23:03PM -0400, Peter Jones wrote: > Make it so that when grub-module-verifier complains of an issue, it tells you > which module the issue was with. > > Signed-off-by: Peter Jones Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Fix an 8 year old typo.
On Wed, Aug 01, 2018 at 12:28:43PM -0400, Peter Jones wrote: > Signed-off-by: Peter Jones Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] module-verifier: make it possible to run checkers on grub-module-verifierxx.c
On Thu, Aug 02, 2018 at 10:36:33AM -0400, Peter Jones wrote: > This makes it so you can treat grub-module-verifierxx.c as a file you can > build directly, so syntax checkers like vim's "syntastic" plugin, which uses > "gcc -x c -fsyntax-only" to build it, will work. > > One still has to do whatever setup is required to make it pick the right > include dirs, which -W options we use, etc., but this makes it so you can do s/-W/-I/? > the checking on the file you're editing, rather than on a different file. > > v2: fix the typo in the #else clause in util/grub-module-verifierXX.c > > Signed-off-by: Peter Jones If apply your patch and call this: gcc -x c -fsyntax-only -Iinclude -I. util/grub-module-verifierXX.c I see following warnings: util/grub-module-verifierXX.c:49:0: warning: "ELF_R_SYM" redefined [enabled by default] In file included from util/grub-module-verifierXX.c:12:0: include/grub/elf.h:2498:0: note: this is the location of the previous definition util/grub-module-verifierXX.c:50:0: warning: "ELF_R_TYPE" redefined [enabled by default] In file included from util/grub-module-verifierXX.c:12:0: include/grub/elf.h:2499:0: note: this is the location of the previous definition util/grub-module-verifierXX.c:51:0: warning: "ELF_ST_TYPE" redefined [enabled by default] In file included from util/grub-module-verifierXX.c:12:0: include/grub/elf.h:2495:0: note: this is the location of the previous definition Should not we drop ELF_R_SYM, ELF_R_TYPE and ELF_ST_TYPE definitions for 64-bits from util/grub-module-verifierXX.c? Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel