Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Critical] The fallback stub __find_fde_lsearch() returns 0 (success) instead of an error when CONFIG_EH_FRAME_LINEAR_SEARCH is disabled, causing the unwinder to consume an uninitialized stack variable. - [High] Enabling CONFIG_EH_FRAME_LINEAR_SEARCH unconditionally triggers linear validation and causes out-of-bounds reads, permanently breaking unwinding for valid binaries lacking a zero terminator. --
--- Patch [16]: [RFC PATCH v1 16/25] unwind_user/eh_frame: Add linear .eh_frame search fallback --- commit 4fc15aee544b13c1c7ff56efb01c804156c21a55 Author: Jens Remus <[email protected]> unwind_user/eh_frame: Add linear .eh_frame search fallback This commit adds a linear search fallback for .eh_frame sections when the .eh_frame_hdr lacks a binary search table. It also introduces validation logic for the referenced .eh_frame section, guarded by a new config option EH_FRAME_LINEAR_SEARCH. > diff --git a/kernel/unwind/eh_frame.c b/kernel/unwind/eh_frame.c > index 7f572d1711d3f..ac288cec8021f 100644 > --- a/kernel/unwind/eh_frame.c > +++ b/kernel/unwind/eh_frame.c [ ... ] > @@ -590,6 +589,82 @@ static __always_inline int __find_fde(struct > eh_frame_section *sec, > return -EFAULT; > } > > +#ifdef CONFIG_EH_FRAME_LINEAR_SEARCH > + > +static __always_inline int __find_fde_lsearch(struct eh_frame_section *sec, > + unsigned long ip, > + struct eh_frame_fde *fde) > +{ > + unsigned long start = sec->eh_frame_start; > + unsigned long vma_end = sec->eh_frame_vma_end; > + unsigned long cur; > + int ret; > + > + /* Linear search through .eh_frame */ > + cur = start; > + while (cur >= start && cur < vma_end) { [Severity: High] Since the section header only provides the start address, this loop iterates all the way to vma_end. As noted in the commit message, many binaries lack a zero terminator. If an unresolved IP is scanned, will this read past the actual section boundary into padding or other sections, return -EINVAL on garbage data, and subsequently cause the caller to permanently remove the section? [ ... ] > + cur = entry_end; > + } > + > + return -ENOENT; > +} > + > +#else /* !CONFIG_EH_FRAME_LINEAR_SEARCH */ > + > +static __always_inline int __find_fde_lsearch(struct eh_frame_section *sec, > + unsigned long ip, > + struct eh_frame_fde *fde) > +{ > + return 0; > +} [Severity: Critical] Does returning 0 here falsely indicate success when linear search is disabled? If a binary lacks a search table, __find_fde() will call this stub. Returning 0 causes the caller, eh_frame_find(), to proceed with an uninitialized fde stack variable: kernel/unwind/eh_frame.c:eh_frame_find() { struct eh_frame_fde fde; ... ret = __find_fde(sec, ip, &fde); if (!ret) ret = __find_frame_row(sec, &fde, ip, frame); ... } Could this lead to corrupted stack traces or arbitrary memory reads by consuming the uninitialized fde context? Should this return -ENOENT instead? [ ... ] > @@ -1246,6 +1321,90 @@ static int eh_frame_validate_section(struct > eh_frame_section *sec) > return -EFAULT; > } > > +static int eh_frame_validate_eh_frame(struct eh_frame_section *sec) > +{ [ ... ] > +static int eh_frame_validate_section(struct eh_frame_section *sec) > +{ > + int ret; > + > + /* > + * Validate .eh_frame_hdr binary search table > + * (incl. all referenced FDE and CIE in .eh_frame). > + */ > + ret = eh_frame_validate_eh_frame_hdr(sec); > + if (ret) > + return ret; > + > + /* > + * Validate .eh_frame CIE and FDE. Skip if linear search > + * is disabled, as many .eh_frame sections lack a zero > + * terminator and the section end if unknown. > + */ > + if (IS_ENABLED(CONFIG_EH_FRAME_LINEAR_SEARCH)) { > + ret = eh_frame_validate_eh_frame(sec); > + if (ret) > + return ret; > + } [Severity: High] Does this unconditionally apply linear validation to all binaries when CONFIG_EH_FRAME_LINEAR_SEARCH is enabled, even if they already have a valid binary search table? If the binary lacks a zero terminator, eh_frame_validate_eh_frame() will read out of bounds up to vma_end, fail with -EINVAL, and reject the entire section. Will this permanently break unwinding for valid binaries that were previously working fine via the binary search table? > + > + return 0; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=16
