| From a1845e90fc19fb5e904091bad8a378f458798e4a Mon Sep 17 00:00:00 2001 |
| From: Peter Jones <pjones@redhat.com> |
| Date: Sun, 19 Jul 2020 15:48:20 -0400 |
| Subject: [PATCH] lvm: Fix two more potential data-dependent alloc |
| overflows |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| 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> |
| Signed-off-by: Stefan SΓΈrensen <stefan.sorensen@spectralink.com> |
| --- |
| grub-core/disk/lvm.c | 48 ++++++++++++++++++++++++++++++++++++-------- |
| 1 file changed, 40 insertions(+), 8 deletions(-) |
| |
| diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c |
| index d1df640b3..139fafd47 100644 |
| --- a/grub-core/disk/lvm.c |
| +++ b/grub-core/disk/lvm.c |
| @@ -25,6 +25,7 @@ |
| #include <grub/lvm.h> |
| #include <grub/partition.h> |
| #include <grub/i18n.h> |
| +#include <grub/safemath.h> |
| |
| #ifdef GRUB_UTIL |
| #include <grub/emu/misc.h> |
| @@ -102,10 +103,11 @@ grub_lvm_detect (grub_disk_t disk, |
| { |
| grub_err_t err; |
| grub_uint64_t mda_offset, mda_size; |
| + grub_size_t ptr; |
| char buf[GRUB_LVM_LABEL_SIZE]; |
| char vg_id[GRUB_LVM_ID_STRLEN+1]; |
| char pv_id[GRUB_LVM_ID_STRLEN+1]; |
| - char *metadatabuf, *p, *q, *vgname; |
| + char *metadatabuf, *p, *q, *mda_end, *vgname; |
| struct grub_lvm_label_header *lh = (struct grub_lvm_label_header *) buf; |
| struct grub_lvm_pv_header *pvh; |
| struct grub_lvm_disk_locn *dlocn; |
| @@ -205,19 +207,31 @@ grub_lvm_detect (grub_disk_t disk, |
| grub_le_to_cpu64 (rlocn->size) - |
| grub_le_to_cpu64 (mdah->size)); |
| } |
| - p = q = metadatabuf + grub_le_to_cpu64 (rlocn->offset); |
| |
| - while (*q != ' ' && q < metadatabuf + mda_size) |
| - q++; |
| - |
| - if (q == metadatabuf + mda_size) |
| + if (grub_add ((grub_size_t)metadatabuf, |
| + (grub_size_t)grub_le_to_cpu64 (rlocn->offset), |
| + &ptr)) |
| { |
| + error_parsing_metadata: |
| #ifdef GRUB_UTIL |
| grub_util_info ("error parsing metadata"); |
| #endif |
| goto fail2; |
| } |
| |
| + p = q = (char *)ptr; |
| + |
| + if (grub_add ((grub_size_t)metadatabuf, (grub_size_t)mda_size, &ptr)) |
| + goto error_parsing_metadata; |
| + |
| + mda_end = (char *)ptr; |
| + |
| + while (*q != ' ' && q < mda_end) |
| + q++; |
| + |
| + if (q == mda_end) |
| + goto error_parsing_metadata; |
| + |
| vgname_len = q - p; |
| vgname = grub_malloc (vgname_len + 1); |
| if (!vgname) |
| @@ -367,8 +381,26 @@ grub_lvm_detect (grub_disk_t disk, |
| { |
| const char *iptr; |
| char *optr; |
| - lv->fullname = grub_malloc (sizeof ("lvm/") - 1 + 2 * vgname_len |
| - + 1 + 2 * s + 1); |
| + |
| + /* |
| + * This is kind of hard to read with our safe (but rather |
| + * baroque) math primatives, but it boils down to: |
| + * |
| + * sz0 = vgname_len * 2 + 1 + |
| + * s * 2 + 1 + |
| + * sizeof ("lvm/") - 1; |
| + */ |
| + grub_size_t sz0 = vgname_len, sz1 = s; |
| + |
| + if (grub_mul (sz0, 2, &sz0) || |
| + grub_add (sz0, 1, &sz0) || |
| + grub_mul (sz1, 2, &sz1) || |
| + grub_add (sz1, 1, &sz1) || |
| + grub_add (sz0, sz1, &sz0) || |
| + grub_add (sz0, sizeof ("lvm/") - 1, &sz0)) |
| + goto lvs_fail; |
| + |
| + lv->fullname = grub_malloc (sz0); |
| if (!lv->fullname) |
| goto lvs_fail; |
| |
| -- |
| 2.26.2 |
| |