Re: [PATCH v3 5/7] x86/microcode/hygon: Add microcode loading support for Hygon processors

[Fu Hao]

On 6/21/2026 8:19 AM, XIAO WU wrote:
> Hi Fu Hao,
>
> I came across a Sashiko AI code review [1] that flagged a potential
> out-of-bounds read in `parse_and_cache_patches()` in the new Hygon
> microcode loader.  I was able to reproduce it in QEMU with KASAN
> enabled.  The trigger is simple: feed a firmware container that has
> only an equivalence table (no patch sections), and the subsequent
> `hygon_section_hdr()` call reads past the buffer end.
>
> On Tue, Jun 16, 2026 at 12:05:29AM +0800, Fu Hao wrote:
>  > Add support for loading Hygon microcode.
> ...
>  > +static enum ucode_state parse_and_cache_patches(u8 family, const u8 
> *data, size_t size)
>  > +{
>  > +    u8 *fw = (u8 *)data;
>  > +    size_t offset;
>  > +
>  > +    offset = load_equiv_cpu_table(data, size);
>  > +    if (!offset)
>  > +        return UCODE_ERROR;
>  > +
>  > +    fw   += offset;
>  > +    size -= offset;
>  > +
>  > +    if (hygon_section_hdr(fw, TYPE) != UCODE_UCODE_TYPE) {
>
> If the container has only an equivalence table and no patch sections,
> `load_equiv_cpu_table()` returns an offset equal to the total buffer
> size.  Then `size -= offset` makes `size == 0`, and `fw` points past
> the buffer.  The call to `hygon_section_hdr(fw, TYPE)` then reads a
> 4-byte section type field from beyond the buffer boundary.
>
> [Reproduction]
>
> I rebuilt the kernel with CONFIG_CPU_SUP_HYGON=y and CONFIG_KASAN=y,
> and added a /proc/hygon_poc entry that calls parse_and_cache_patches()
> with a crafted firmware blob containing only an equivalence table
> (zero patch sections).  Writing any value to /proc/hygon_poc triggers
> the path.
>
> PoC source (poc.c):
> ---8<----------------------------------------------------------------
> /*
>   * PoC: OOB read in parse_and_cache_patches() in hygon.c
>   * Trigger: echo X > /proc/hygon_poc (or run this binary)
>   */
> #include <stdio.h>
> #include <stdlib.h>
> #include <fcntl.h>
> #include <unistd.h>
>
> int main(void)
> {
>      int fd = open("/proc/hygon_poc", O_WRONLY);
>      if (fd < 0) { perror("open"); return 1; }
>      write(fd, "X", 1);
>      close(fd);
>      printf("Done. Check dmesg.\n");
>      return 0;
> }
> ---8<----------------------------------------------------------------
> Compile: gcc -o poc poc.c -static
>
> [KASAN report — kernel 6.19.0-rc5, CONFIG_KASAN=y]
>
>    ==================================================================
>    BUG: KASAN: stack-out-of-bounds in 
> parse_and_cache_patches.isra.0+0x805/0x8e0
>    Read of size 4 at addr ffffc9002106fc74 by task poc/9404
>
>    CPU: 1 PID: 9404 Comm: poc Tainted: G  W 6.19.0-rc5-gd3caa0b6e4ac- 
> dirty #2
>
>    Call Trace:
>     <TASK>
>     dump_stack_lvl+0x116/0x1f0
>     print_report+0xcd/0x630
>     kasan_report+0xe0/0x110
>     parse_and_cache_patches.isra.0+0x805/0x8e0
>     hygon_poc_write+0x97/0xd0
>     proc_reg_write+0x245/0x340
>     vfs_write+0x2a5/0x11e0
>     ksys_write+0x12f/0x250
>     do_syscall_64+0xcd/0xf80
>     entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
>    The buggy address belongs to stack of task poc/9404
>     and is located at offset 52 in frame:
>     hygon_poc_write+0x0/0xd0
>     This frame has 1 object: [32, 52) 'test_hdr'
>
> The crash is at `parse_and_cache_patches+0x805`, which corresponds to
> the `hygon_section_hdr(fw, TYPE)` call reading the section type from
> beyond the buffer.  The stack object `test_hdr` at [32, 52) has size
> 20 bytes — the OOB read at offset 52 is exactly at the boundary.
>
> [Suggested fix]
>
> Add a bounds check before reading the section header:
>
>    offset = load_equiv_cpu_table(data, size);
>    if (!offset)
>        return UCODE_ERROR;
>
>    fw   += offset;
>    size -= offset;
>
> + if (size < HYGON_SECTION_HDR_SIZE) {
> +     pr_err("no patch sections in container\n");
> +     return UCODE_ERROR;
> + }
> +
>    if (hygon_section_hdr(fw, TYPE) != UCODE_UCODE_TYPE) {
>
> This mirrors the pattern used in the AMD loader
> (parse_container() in amd.c), which checks `size < 0` before
> accessing section headers.
>
> The review also notes that `kzalloc_obj` in `try_verify_and_cache_patch()`
> does not appear to be a defined kernel macro, and `hypervisor_present`
> may not be declared — these are build-time issues separate from the
> runtime OOB above.
>
> [1] https://sashiko.dev/#/patchset/cover.1781523812.git.fuhao%40open- 
> hieco.net
>      (Sashiko AI code review — "Out-of-Bounds Access", Severity: High)
>
> Thanks,
> XIAOWU

Hi Xiao Wu,

   Thank you for your review and suggestions. You are right, this could
indeed cause issues. I will fix it in v4 and add more comprehensive
tests to improve coverage.

--
Regards,
Fu Hao