It appears to be possible to make a (possibly invalid) lvm PV with
a metadata size field that overflows our type when adding it to the
address we've allocated. Even if it doesn't, it may be possible to do so
with the math using the outcome of that as an operand. Check them both.
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This modifies most of the places we do some form of:
X = malloc(Y * Z);
to use calloc(Y, Z) instead.
Among other issues, this fixes:
- allocation of integer overflow in grub_png_decode_image_header()
reported by Chris Coulson,
- allocation of integer overflow in luks_recover_key()
reported by Chris Coulson,
- allocation of integer overflow in grub_lvm_detect()
reported by Chris Coulson.
Fixes: CVE-2020-14308
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The LVM cache logical volume is the logical volume consisting of the original
and the cache pool logical volume. The original is usually on a larger and
slower storage device while the cache pool is on a smaller and faster one. The
performance of the original volume can be improved by storing the frequently
used data on the cache pool to utilize the greater performance of faster
device.
The default cache mode "writethrough" ensures that any data written will be
stored both in the cache and on the origin LV, therefore grub can be straight
to read the original lv as no data loss is guarenteed.
The second cache mode is "writeback", which delays writing from the cache pool
back to the origin LV to have increased performance. The drawback is potential
data loss if losing the associated cache device.
During the boot time grub reads the LVM offline i.e. LVM volumes are not
activated and mounted, hence it should be fine to read directly from original
lv since all cached data should have been flushed back in the process of taking
it offline.
It is also not much helpful to the situation by adding fsync calls to the
install code. The fsync did not force to write back dirty cache to the original
device and rather it would update associated cache metadata to complete the
write transaction with the cache device. IOW the writes to cached blocks still
go only to the cache device.
To write back dirty cache, as LVM cache did not support dirty cache flush per
block range, there'no way to do it for file. On the other hand the "cleaner"
policy is implemented and can be used to write back "all" dirty blocks in a
cache, which effectively drain all dirty cache gradually to attain and last in
the "clean" state, which can be useful for shrinking or decommissioning a
cache. The result and effect is not what we are looking for here.
In conclusion, as it seems no way to enforce file writes to the original
device, grub may suffer from power failure as it cannot assemble the cache
device and read the dirty data from it. However since the case is only
applicable to writeback mode which is sensitive to data lost in nature, I'd
still like to propose my (relatively simple) patch and treat reading dirty
cache as improvement.
Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Currently the string functions grub_strtol(), grub_strtoul(), and
grub_strtoull() don't declare the "end" pointer in such a way as to
require the pointer itself or the character array to be immutable to the
implementation, nor does the C standard do so in its similar functions,
though it does require us not to change any of it.
The typical declarations of these functions follow this pattern:
long
strtol(const char * restrict nptr, char ** restrict endptr, int base);
Much of the reason for this is historic, and a discussion of that
follows below, after the explanation of this change. (GRUB currently
does not include the "restrict" qualifiers, and we name the arguments a
bit differently.)
The implementation is semantically required to treat the character array
as immutable, but such accidental modifications aren't stopped by the
compiler, and the semantics for both the callers and the implementation
of these functions are sometimes also helped by adding that requirement.
This patch changes these declarations to follow this pattern instead:
long
strtol(const char * restrict nptr,
const char ** const restrict endptr,
int base);
This means that if any modification to these functions accidentally
introduces either an errant modification to the underlying character
array, or an accidental assignment to endptr rather than *endptr, the
compiler should generate an error. (The two uses of "restrict" in this
case basically mean strtol() isn't allowed to modify the character array
by going through *endptr, and endptr isn't allowed to point inside the
array.)
It also means the typical use case changes to:
char *s = ...;
const char *end;
long l;
l = strtol(s, &end, 10);
Or even:
const char *p = str;
while (p && *p) {
long l = strtol(p, &p, 10);
...
}
This fixes 26 places where we discard our attempts at treating the data
safely by doing:
const char *p = str;
long l;
l = strtol(p, (char **)&ptr, 10);
It also adds 5 places where we do:
char *p = str;
while (p && *p) {
long l = strtol(p, (const char ** const)&p, 10);
...
/* more calls that need p not to be pointer-to-const */
}
While moderately distasteful, this is a better problem to have.
With one minor exception, I have tested that all of this compiles
without relevant warnings or errors, and that /much/ of it behaves
correctly, with gcc 9 using 'gcc -W -Wall -Wextra'. The one exception
is the changes in grub-core/osdep/aros/hostdisk.c , which I have no idea
how to build.
Because the C standard defined type-qualifiers in a way that can be
confusing, in the past there's been a slow but fairly regular stream of
churn within our patches, which add and remove the const qualifier in many
of the users of these functions. This change should help avoid that in
the future, and in order to help ensure this, I've added an explanation
in misc.h so that when someone does get a compiler warning about a type
error, they have the fix at hand.
The reason we don't have "const" in these calls in the standard is
purely anachronistic: C78 (de facto) did not have type qualifiers in the
syntax, and the "const" type qualifier was added for C89 (I think; it
may have been later). strtol() appears to date from 4.3BSD in 1986,
which means it could not be added to those functions in the standard
without breaking compatibility, which is usually avoided.
The syntax chosen for type qualifiers is what has led to the churn
regarding usage of const, and is especially confusing on string
functions due to the lack of a string type. Quoting from C99, the
syntax is:
declarator:
pointer[opt] direct-declarator
direct-declarator:
identifier
( declarator )
direct-declarator [ type-qualifier-list[opt] assignment-expression[opt] ]
...
direct-declarator [ type-qualifier-list[opt] * ]
...
pointer:
* type-qualifier-list[opt]
* type-qualifier-list[opt] pointer
type-qualifier-list:
type-qualifier
type-qualifier-list type-qualifier
...
type-qualifier:
const
restrict
volatile
So the examples go like:
const char foo; // immutable object
const char *foo; // mutable pointer to object
char * const foo; // immutable pointer to mutable object
const char * const foo; // immutable pointer to immutable object
const char const * const foo; // XXX extra const keyword in the middle
const char * const * const foo; // immutable pointer to immutable
// pointer to immutable object
const char ** const foo; // immutable pointer to mutable pointer
// to immutable object
Making const left-associative for * and right-associative for everything
else may not have been the best choice ever, but here we are, and the
inevitable result is people using trying to use const (as they should!),
putting it at the wrong place, fighting with the compiler for a bit, and
then either removing it or typecasting something in a bad way. I won't
go into describing restrict, but its syntax has exactly the same issue
as with const.
Anyway, the last example above actually represents the *behavior* that's
required of strtol()-like functions, so that's our choice for the "end"
pointer.
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
* include/grub/diskfilter.h (grub_diskfilter_register): Renamed to ..
(grub_diskfilter_register_front): ... this.
(grub_diskfilter_register_back): New function.
All users of grub_diskfilter_register updated.
* Makefile.util.def (libgrubkern.a): Add grub-core/kern/emu/raid.c.
(grub-setup): Remove util/raid.c.
* grub-core/Makefile.core.def (kernel): Add kern/emu/raid.c on emu.
* grub-core/disk/lvm.c (scan_depth): New variable.
(grub_lvm_iterate): Rescan if necessary.
(find_lv): New function based on grub_lvm_open.
(grub_lvm_open): Use find_lv. Rescan on error.
(is_node_readable): New function.
(is_lv_readable): Likewise.
(grub_lvm_scan_device): Skip already found disks.
(do_lvm_scan): New function. Move grub_lvm_scan_device inside of it.
Stop if searched device is found and readable.
* grub-core/disk/raid.c (inscnt): New variable.
(scan_depth): Likewise.
(scan_devices): New function based on grub_raid_register. Abort if
looked for device is found.
(grub_raid_iterate): Rescan if needed.
(find_array): NEw function based on -grub_raid_open.
(grub_raid_open): Use find_array and rescan.
(insert_array): Set became_readable_at.
* grub-core/kern/disk.c (grub_disk_dev_iterate): Iterate though "pull.
* grub-core/kern/emu/getroot.c (grub_util_open_dm) [HAVE_DEVICE_MAPPER]:
New function.
(grub_util_is_lvm) [HAVE_DEVICE_MAPPER]: Use grub_util_open_dm.
(grub_util_pull_device): New function.
(grub_util_get_grub_dev): Call grub_util_pull_device.
* util/raid.c: Moved to ..
* grub-core/kern/emu/raid.c: ... here.
(grub_util_raid_getmembers): New parameter "bootable".
All users updated. Support 1.x.
* include/grub/ata.h (grub_ata_dev): Change iterate prototype.
All users updated.
* include/grub/disk.h (grub_disk_pull_t): New enum.
(grub_disk_dev): Change iterate prototype.
All users updated.
* include/grub/emu/getroot.h (grub_util_raid_getmembers) [__linux__]:
New proto.
* include/grub/emu/hostdisk.h (grub_util_pull_device): Likewise.
* include/grub/lvm.h (grub_lvm_lv): New members fullname and compatname.
* include/grub/raid.h (grub_raid_array): New member became_readable_at.
* include/grub/scsi.h (grub_scsi_dev): Change iterate prototype.
All users updated.
* include/grub/util/raid.h: Removed.
* grub-core/kern/misc.c (grub_divmod64_full): Renamed to ...
(grub_divmod64): ... this.
* include/grub/misc.h (grub_divmod64): Removed. All users switch to full
version.
configurations like pvmove. Previously code assumed that in some places
only lvs or only pvs are used whereas it seems that they are used
interchangeably.
* grub-core/disk/lvm.c (read_node): New function.
(read_lv): Use read_node.
(grub_lvm_scan_device): Use only first mirror on pvmove'd lvs.
Match volumes only at the end when all lvs are found. Take both
pvs (first) and lvs (second) into account.
* include/grub/lvm.h (grub_lvm_segment): Merge fields stripe_* and
mirror_* into node_*. All users updated.
(grub_lvm_stripe): Merge this ...
(grub_lvm_mirror): ... and this ...
(grub_lvm_node): ... into this. All users updated.
All users updated.
* disk/loopback.c (grub_loopback): Remove has_partitions.
All users updated.
(options): Remove partitions. All users updated.
* util/grub-fstest.c (fstest): Don't pass "-p" to loopback.
* util/i386/pc/grub-setup.c (setup): copy partition table only when
actual partition table is found.
