| Message ID | 20230812014712.6117-1-karn.jye.lau@intel.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kirkstone] Fix kirkstone dmidedecode smbios3_decode | expand |
Thanks for the patch. Unfortunately there is an issue: Applying patch 0002-Fix-smbios3_decode-in-kirkstone-dmidecode.patch patching file dmidecode.c Hunk #1 succeeded at 3513 (offset -1 lines). Hunk #2 succeeded at 5443 (offset -51 lines). Hunk #3 succeeded at 5461 with fuzz 2 (offset -48 lines). Applying patch CVE-2023-30630_1.patch patching file dmidecode.c Hunk #1 succeeded at 5196 (offset -231 lines). Hunk #2 succeeded at 5421 (offset -272 lines). Hunk #3 succeeded at 5456 (offset -272 lines). Hunk #4 succeeded at 5497 with fuzz 2 (offset -269 lines). Hunk #5 succeeded at 5514 (offset -261 lines). Hunk #6 succeeded at 5565 (offset -261 lines). Hunk #7 succeeded at 5573 (offset -261 lines). Hunk #8 succeeded at 5594 (offset -261 lines). patching file util.c patching file util.h Could you please fix the fuzz error and submit a v2? Also, a more standard shortlog would be something like: dmidecode: fix smbios3_decode Could you fix this also with v2? Thanks again! Steve On Fri, Aug 11, 2023 at 3:50 PM Lau, Karn Jye <karn.jye.lau@intel.com> wrote: > > From: "Lau, Karn Jye" <karn.jye.lau@intel.com> > > Recent CVE fixes in kirkstone dmidecode broke it > functionality, this issue is only observed in kirkstone > version of dmidecode(v3.3).Update smbios3_decode to address > the broken functionality. > > Signed-off-by: Lau, Karn Jye <karn.jye.lau@intel.com> > --- > ...mbios3_decode-in-kirkstone-dmidecode.patch | 125 ++++++++++++++++++ > .../dmidecode/dmidecode_3.3.bb | 1 + > 2 files changed, 126 insertions(+) > create mode 100644 meta/recipes-devtools/dmidecode/dmidecode/0002-Fix-smbios3_decode-in-kirkstone-dmidecode.patch > > diff --git a/meta/recipes-devtools/dmidecode/dmidecode/0002-Fix-smbios3_decode-in-kirkstone-dmidecode.patch b/meta/recipes-devtools/dmidecode/dmidecode/0002-Fix-smbios3_decode-in-kirkstone-dmidecode.patch > new file mode 100644 > index 0000000000..00ffb90ce2 > --- /dev/null > +++ b/meta/recipes-devtools/dmidecode/dmidecode/0002-Fix-smbios3_decode-in-kirkstone-dmidecode.patch > @@ -0,0 +1,125 @@ > +From 8a395982d6f350d0744666cffe42c4a486656c6f Mon Sep 17 00:00:00 2001 > +From: "Lau, Karn Jye" <karn.jye.lau@intel.com> > +Date: Sat, 12 Aug 2023 08:41:58 +0800 > +Subject: [PATCH 2/2] Fix smbios3_decode in kirkstone dmidecode > + > +Recent CVE fix broke dmidecode functionality, > +port upstream changes to fix smbios3_decodein > +function. > + > +Reference:https://github.com/mirror/dmidecode/commit/39b2dd7b6ab719b920e96ed832cfb4bdd664e808 > + > +Signed-off-by: Lau, Karn Jye <karn.jye.lau@intel.com> > +--- > + dmidecode.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++-- > + 1 file changed, 79 insertions(+), 2 deletions(-) > + > +diff --git a/dmidecode.c b/dmidecode.c > +index f826f6c..91e1a32 100644 > +--- a/dmidecode.c > ++++ b/dmidecode.c > +@@ -3514,6 +3514,72 @@ static const char *dmi_power_supply_range_switching(u8 code) > + return out_of_spec; > + } > + > ++/* Allocates a buffer for the table, must be freed by the caller */ > ++static u8 *dmi_table_get(off_t base, u32 *len, u16 num, u32 ver, > ++ const char *devmem, u32 flags) > ++{ > ++ u8 *buf; > ++ > ++ if (ver > SUPPORTED_SMBIOS_VER && !(opt.flags & FLAG_QUIET)) > ++ { > ++ pr_comment("SMBIOS implementations newer than version %u.%u.%u are not", > ++ SUPPORTED_SMBIOS_VER >> 16, > ++ (SUPPORTED_SMBIOS_VER >> 8) & 0xFF, > ++ SUPPORTED_SMBIOS_VER & 0xFF); > ++ pr_comment("fully supported by this version of dmidecode."); > ++ } > ++ > ++ if (!(opt.flags & FLAG_QUIET)) > ++ { > ++ if (opt.type == NULL) > ++ { > ++ if (num) > ++ pr_info("%u structures occupying %u bytes.", > ++ num, *len); > ++ if (!(opt.flags & FLAG_FROM_DUMP)) > ++ pr_info("Table at 0x%08llX.", > ++ (unsigned long long)base); > ++ } > ++ pr_sep(); > ++ } > ++ > ++ if ((flags & FLAG_NO_FILE_OFFSET) || (opt.flags & FLAG_FROM_DUMP)) > ++ { > ++ /* > ++ * When reading from sysfs or from a dump file, the file may be > ++ * shorter than announced. For SMBIOS v3 this is expcted, as we > ++ * only know the maximum table size, not the actual table size. > ++ * For older implementations (and for SMBIOS v3 too), this > ++ * would be the result of the kernel truncating the table on > ++ * parse error. > ++ */ > ++ size_t size = *len; > ++ buf = read_file(flags & FLAG_NO_FILE_OFFSET ? 0 : base, > ++ &size, devmem); > ++ if (!(opt.flags & FLAG_QUIET) && num && size != (size_t)*len) > ++ { > ++ fprintf(stderr, "Wrong DMI structures length: %u bytes " > ++ "announced, only %lu bytes available.\n", > ++ *len, (unsigned long)size); > ++ } > ++ *len = size; > ++ } > ++ else > ++ buf = mem_chunk(base, *len, devmem); > ++ > ++ if (buf == NULL) > ++ { > ++ fprintf(stderr, "Failed to read table, sorry.\n"); > ++#ifndef USE_MMAP > ++ if (!(flags & FLAG_NO_FILE_OFFSET)) > ++ fprintf(stderr, > ++ "Try compiling dmidecode with -DUSE_MMAP.\n"); > ++#endif > ++ } > ++ > ++ return buf; > ++} > ++ > + /* > + * 7.41 Additional Information (Type 40) > + * > +@@ -5428,8 +5494,11 @@ static int smbios3_decode(u8 *buf, size_t buf_len, const char *devmem, u32 flags > + return 0; > + } > + > +- dmi_table(((off_t)offset.h << 32) | offset.l, > +- DWORD(buf + 0x0C), 0, ver, devmem, flags | FLAG_STOP_AT_EOT); > ++ /* Maximum length, may get trimmed */ > ++ > ++ len = DWORD(buf + 0x0C); > ++ > ++ table = dmi_table_get(((off_t)offset.h << 32) | offset.l, &len, 0, ver,devmem, flags | FLAG_STOP_AT_EOT); > + > + if (opt.flags & FLAG_DUMP_BIN) > + { > +@@ -5440,6 +5509,14 @@ static int smbios3_decode(u8 *buf, size_t buf_len, const char *devmem, u32 flags > + > + dmi_table_dump(crafted, crafted[0x06], table, len); > + } > ++ else > ++ { > ++ dmi_table_decode(table, len, 0, ver >> 8,flags | FLAG_STOP_AT_EOT); > ++ } > ++ > ++ > ++ > ++ free(table); > + > + return 1; > + } > +-- > +2.34.1 > + > diff --git a/meta/recipes-devtools/dmidecode/dmidecode_3.3.bb b/meta/recipes-devtools/dmidecode/dmidecode_3.3.bb > index b99c2ea99d..fab7a9ec97 100644 > --- a/meta/recipes-devtools/dmidecode/dmidecode_3.3.bb > +++ b/meta/recipes-devtools/dmidecode/dmidecode_3.3.bb > @@ -6,6 +6,7 @@ LIC_FILES_CHKSUM = "file://LICENSE;md5=b234ee4d69f5fce4486a80fdaf4a4263" > > SRC_URI = "${SAVANNAH_NONGNU_MIRROR}/dmidecode/${BP}.tar.xz \ > file://0001-Committing-changes-from-do_unpack_extra.patch \ > + file://0002-Fix-smbios3_decode-in-kirkstone-dmidecode.patch \ > file://CVE-2023-30630_1.patch \ > file://CVE-2023-30630_2.patch \ > file://CVE-2023-30630_3.patch \ > -- > 2.34.1 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#185854): https://lists.openembedded.org/g/openembedded-core/message/185854 > Mute This Topic: https://lists.openembedded.org/mt/100696063/3620601 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [steve@sakoman.com] > -=-=-=-=-=-=-=-=-=-=-=- >
On Sat, 2023-08-12 at 09:47 +0800, Lau, Karn Jye wrote: > From: "Lau, Karn Jye" <karn.jye.lau@intel.com> > > Recent CVE fixes in kirkstone dmidecode broke it > functionality, this issue is only observed in kirkstone > version of dmidecode(v3.3).Update smbios3_decode to address > the broken functionality. > > Signed-off-by: Lau, Karn Jye <karn.jye.lau@intel.com> > --- > ...mbios3_decode-in-kirkstone-dmidecode.patch | 125 > ++++++++++++++++++ > .../dmidecode/dmidecode_3.3.bb | 1 + > 2 files changed, 126 insertions(+) > create mode 100644 meta/recipes-devtools/dmidecode/dmidecode/0002- > Fix-smbios3_decode-in-kirkstone-dmidecode.patch > > diff --git a/meta/recipes-devtools/dmidecode/dmidecode/0002-Fix- > smbios3_decode-in-kirkstone-dmidecode.patch b/meta/recipes- > devtools/dmidecode/dmidecode/0002-Fix-smbios3_decode-in-kirkstone- > dmidecode.patch > new file mode 100644 > index 0000000000..00ffb90ce2 > --- /dev/null > +++ b/meta/recipes-devtools/dmidecode/dmidecode/0002-Fix- > smbios3_decode-in-kirkstone-dmidecode.patch > @@ -0,0 +1,125 @@ > +From 8a395982d6f350d0744666cffe42c4a486656c6f Mon Sep 17 00:00:00 > 2001 > +From: "Lau, Karn Jye" <karn.jye.lau@intel.com> > +Date: Sat, 12 Aug 2023 08:41:58 +0800 > +Subject: [PATCH 2/2] Fix smbios3_decode in kirkstone dmidecode > + > +Recent CVE fix broke dmidecode functionality, > +port upstream changes to fix smbios3_decodein > +function. > + > +Reference: > https://github.com/mirror/dmidecode/commit/39b2dd7b6ab719b920e96ed832 > cfb4bdd664e808 Why are we backporting only a part of this commit? Thanks, Anuj > + > +Signed-off-by: Lau, Karn Jye <karn.jye.lau@intel.com> > +--- > + dmidecode.c | 81 > +++++++++++++++++++++++++++++++++++++++++++++++++++-- > + 1 file changed, 79 insertions(+), 2 deletions(-) > + > +diff --git a/dmidecode.c b/dmidecode.c > +index f826f6c..91e1a32 100644 > +--- a/dmidecode.c > ++++ b/dmidecode.c > +@@ -3514,6 +3514,72 @@ static const char > *dmi_power_supply_range_switching(u8 code) > + return out_of_spec; > + } > + > ++/* Allocates a buffer for the table, must be freed by the caller */ > ++static u8 *dmi_table_get(off_t base, u32 *len, u16 num, u32 ver, > ++ const char *devmem, u32 flags) > ++{ > ++ u8 *buf; > ++ > ++ if (ver > SUPPORTED_SMBIOS_VER && !(opt.flags & FLAG_QUIET)) > ++ { > ++ pr_comment("SMBIOS implementations newer than version > %u.%u.%u are not", > ++ SUPPORTED_SMBIOS_VER >> 16, > ++ (SUPPORTED_SMBIOS_VER >> 8) & 0xFF, > ++ SUPPORTED_SMBIOS_VER & 0xFF); > ++ pr_comment("fully supported by this version of > dmidecode."); > ++ } > ++ > ++ if (!(opt.flags & FLAG_QUIET)) > ++ { > ++ if (opt.type == NULL) > ++ { > ++ if (num) > ++ pr_info("%u structures occupying %u > bytes.", > ++ num, *len); > ++ if (!(opt.flags & FLAG_FROM_DUMP)) > ++ pr_info("Table at 0x%08llX.", > ++ (unsigned long long)base); > ++ } > ++ pr_sep(); > ++ } > ++ > ++ if ((flags & FLAG_NO_FILE_OFFSET) || (opt.flags & > FLAG_FROM_DUMP)) > ++ { > ++ /* > ++ * When reading from sysfs or from a dump file, the > file may be > ++ * shorter than announced. For SMBIOS v3 this is > expcted, as we > ++ * only know the maximum table size, not the actual > table size. > ++ * For older implementations (and for SMBIOS v3 too), > this > ++ * would be the result of the kernel truncating the > table on > ++ * parse error. > ++ */ > ++ size_t size = *len; > ++ buf = read_file(flags & FLAG_NO_FILE_OFFSET ? 0 : > base, > ++ &size, devmem); > ++ if (!(opt.flags & FLAG_QUIET) && num && size != > (size_t)*len) > ++ { > ++ fprintf(stderr, "Wrong DMI structures length: > %u bytes " > ++ "announced, only %lu bytes > available.\n", > ++ *len, (unsigned long)size); > ++ } > ++ *len = size; > ++ } > ++ else > ++ buf = mem_chunk(base, *len, devmem); > ++ > ++ if (buf == NULL) > ++ { > ++ fprintf(stderr, "Failed to read table, sorry.\n"); > ++#ifndef USE_MMAP > ++ if (!(flags & FLAG_NO_FILE_OFFSET)) > ++ fprintf(stderr, > ++ "Try compiling dmidecode with - > DUSE_MMAP.\n"); > ++#endif > ++ } > ++ > ++ return buf; > ++} > ++ > + /* > + * 7.41 Additional Information (Type 40) > + * > +@@ -5428,8 +5494,11 @@ static int smbios3_decode(u8 *buf, size_t > buf_len, const char *devmem, u32 flags > + return 0; > + } > + > +- dmi_table(((off_t)offset.h << 32) | offset.l, > +- DWORD(buf + 0x0C), 0, ver, devmem, flags | > FLAG_STOP_AT_EOT); > ++ /* Maximum length, may get trimmed */ > ++ > ++ len = DWORD(buf + 0x0C); > ++ > ++ table = dmi_table_get(((off_t)offset.h << 32) | offset.l, &len, > 0, ver,devmem, flags | FLAG_STOP_AT_EOT); > + > + if (opt.flags & FLAG_DUMP_BIN) > + { > +@@ -5440,6 +5509,14 @@ static int smbios3_decode(u8 *buf, size_t > buf_len, const char *devmem, u32 flags > + > + dmi_table_dump(crafted, crafted[0x06], table, len); > + } > ++ else > ++ { > ++ dmi_table_decode(table, len, 0, ver >> 8,flags | > FLAG_STOP_AT_EOT); > ++ } > ++ > ++ > ++ > ++ free(table); > + > + return 1; > + } > +-- > +2.34.1 > + > diff --git a/meta/recipes-devtools/dmidecode/dmidecode_3.3.bb > b/meta/recipes-devtools/dmidecode/dmidecode_3.3.bb > index b99c2ea99d..fab7a9ec97 100644 > --- a/meta/recipes-devtools/dmidecode/dmidecode_3.3.bb > +++ b/meta/recipes-devtools/dmidecode/dmidecode_3.3.bb > @@ -6,6 +6,7 @@ LIC_FILES_CHKSUM = > "file://LICENSE;md5=b234ee4d69f5fce4486a80fdaf4a4263" > > SRC_URI = "${SAVANNAH_NONGNU_MIRROR}/dmidecode/${BP}.tar.xz \ > > file://0001-Committing-changes-from-do_unpack_extra.patch \ > + > file://0002-Fix-smbios3_decode-in-kirkstone-dmidecode.patch \ > file://CVE-2023-30630_1.patch \ > file://CVE-2023-30630_2.patch \ > file://CVE-2023-30630_3.patch \ > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#185854): > https://lists.openembedded.org/g/openembedded-core/message/185854 > Mute This Topic: https://lists.openembedded.org/mt/100696063/3616702 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: > https://lists.openembedded.org/g/openembedded-core/unsub [ > anuj.mittal@intel.com] > -=-=-=-=-=-=-=-=-=-=-=- >
Here is a fix for this issue: https://lists.openembedded.org/g/openembedded-core/message/186054. Unfortunately, I was not able to link to this discussion.
On Tue, Aug 15, 2023 at 12:02 AM Adrian Freihofer <adrian.freihofer@gmail.com> wrote: > > Here is a fix for this issue: https://lists.openembedded.org/g/openembedded-core/message/186054. Unfortunately, I was not able to link to this discussion. I have this version of the fix in my current test queue. Steve > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#186056): https://lists.openembedded.org/g/openembedded-core/message/186056 > Mute This Topic: https://lists.openembedded.org/mt/100696063/3620601 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [steve@sakoman.com] > -=-=-=-=-=-=-=-=-=-=-=- >
Looking at this patch set again, I just found a mistake: I forgot to add patch 5 to the SRC_URI. I will send a v2 as soon as possible. This one will have 2 improvements: - Adding all patches to the SRC_URI. - Renaming the patches to their original names to minimize differences and simplify review.
diff --git a/meta/recipes-devtools/dmidecode/dmidecode/0002-Fix-smbios3_decode-in-kirkstone-dmidecode.patch b/meta/recipes-devtools/dmidecode/dmidecode/0002-Fix-smbios3_decode-in-kirkstone-dmidecode.patch new file mode 100644 index 0000000000..00ffb90ce2 --- /dev/null +++ b/meta/recipes-devtools/dmidecode/dmidecode/0002-Fix-smbios3_decode-in-kirkstone-dmidecode.patch @@ -0,0 +1,125 @@ +From 8a395982d6f350d0744666cffe42c4a486656c6f Mon Sep 17 00:00:00 2001 +From: "Lau, Karn Jye" <karn.jye.lau@intel.com> +Date: Sat, 12 Aug 2023 08:41:58 +0800 +Subject: [PATCH 2/2] Fix smbios3_decode in kirkstone dmidecode + +Recent CVE fix broke dmidecode functionality, +port upstream changes to fix smbios3_decodein +function. + +Reference:https://github.com/mirror/dmidecode/commit/39b2dd7b6ab719b920e96ed832cfb4bdd664e808 + +Signed-off-by: Lau, Karn Jye <karn.jye.lau@intel.com> +--- + dmidecode.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++-- + 1 file changed, 79 insertions(+), 2 deletions(-) + +diff --git a/dmidecode.c b/dmidecode.c +index f826f6c..91e1a32 100644 +--- a/dmidecode.c ++++ b/dmidecode.c +@@ -3514,6 +3514,72 @@ static const char *dmi_power_supply_range_switching(u8 code) + return out_of_spec; + } + ++/* Allocates a buffer for the table, must be freed by the caller */ ++static u8 *dmi_table_get(off_t base, u32 *len, u16 num, u32 ver, ++ const char *devmem, u32 flags) ++{ ++ u8 *buf; ++ ++ if (ver > SUPPORTED_SMBIOS_VER && !(opt.flags & FLAG_QUIET)) ++ { ++ pr_comment("SMBIOS implementations newer than version %u.%u.%u are not", ++ SUPPORTED_SMBIOS_VER >> 16, ++ (SUPPORTED_SMBIOS_VER >> 8) & 0xFF, ++ SUPPORTED_SMBIOS_VER & 0xFF); ++ pr_comment("fully supported by this version of dmidecode."); ++ } ++ ++ if (!(opt.flags & FLAG_QUIET)) ++ { ++ if (opt.type == NULL) ++ { ++ if (num) ++ pr_info("%u structures occupying %u bytes.", ++ num, *len); ++ if (!(opt.flags & FLAG_FROM_DUMP)) ++ pr_info("Table at 0x%08llX.", ++ (unsigned long long)base); ++ } ++ pr_sep(); ++ } ++ ++ if ((flags & FLAG_NO_FILE_OFFSET) || (opt.flags & FLAG_FROM_DUMP)) ++ { ++ /* ++ * When reading from sysfs or from a dump file, the file may be ++ * shorter than announced. For SMBIOS v3 this is expcted, as we ++ * only know the maximum table size, not the actual table size. ++ * For older implementations (and for SMBIOS v3 too), this ++ * would be the result of the kernel truncating the table on ++ * parse error. ++ */ ++ size_t size = *len; ++ buf = read_file(flags & FLAG_NO_FILE_OFFSET ? 0 : base, ++ &size, devmem); ++ if (!(opt.flags & FLAG_QUIET) && num && size != (size_t)*len) ++ { ++ fprintf(stderr, "Wrong DMI structures length: %u bytes " ++ "announced, only %lu bytes available.\n", ++ *len, (unsigned long)size); ++ } ++ *len = size; ++ } ++ else ++ buf = mem_chunk(base, *len, devmem); ++ ++ if (buf == NULL) ++ { ++ fprintf(stderr, "Failed to read table, sorry.\n"); ++#ifndef USE_MMAP ++ if (!(flags & FLAG_NO_FILE_OFFSET)) ++ fprintf(stderr, ++ "Try compiling dmidecode with -DUSE_MMAP.\n"); ++#endif ++ } ++ ++ return buf; ++} ++ + /* + * 7.41 Additional Information (Type 40) + * +@@ -5428,8 +5494,11 @@ static int smbios3_decode(u8 *buf, size_t buf_len, const char *devmem, u32 flags + return 0; + } + +- dmi_table(((off_t)offset.h << 32) | offset.l, +- DWORD(buf + 0x0C), 0, ver, devmem, flags | FLAG_STOP_AT_EOT); ++ /* Maximum length, may get trimmed */ ++ ++ len = DWORD(buf + 0x0C); ++ ++ table = dmi_table_get(((off_t)offset.h << 32) | offset.l, &len, 0, ver,devmem, flags | FLAG_STOP_AT_EOT); + + if (opt.flags & FLAG_DUMP_BIN) + { +@@ -5440,6 +5509,14 @@ static int smbios3_decode(u8 *buf, size_t buf_len, const char *devmem, u32 flags + + dmi_table_dump(crafted, crafted[0x06], table, len); + } ++ else ++ { ++ dmi_table_decode(table, len, 0, ver >> 8,flags | FLAG_STOP_AT_EOT); ++ } ++ ++ ++ ++ free(table); + + return 1; + } +-- +2.34.1 + diff --git a/meta/recipes-devtools/dmidecode/dmidecode_3.3.bb b/meta/recipes-devtools/dmidecode/dmidecode_3.3.bb index b99c2ea99d..fab7a9ec97 100644 --- a/meta/recipes-devtools/dmidecode/dmidecode_3.3.bb +++ b/meta/recipes-devtools/dmidecode/dmidecode_3.3.bb @@ -6,6 +6,7 @@ LIC_FILES_CHKSUM = "file://LICENSE;md5=b234ee4d69f5fce4486a80fdaf4a4263" SRC_URI = "${SAVANNAH_NONGNU_MIRROR}/dmidecode/${BP}.tar.xz \ file://0001-Committing-changes-from-do_unpack_extra.patch \ + file://0002-Fix-smbios3_decode-in-kirkstone-dmidecode.patch \ file://CVE-2023-30630_1.patch \ file://CVE-2023-30630_2.patch \ file://CVE-2023-30630_3.patch \