Skip to content

events-arch-x86: fix an uninitialized bank_type and an mce_priv leak - #267

Open
agenticode wants to merge 2 commits into
mchehab:masterfrom
agenticode:x86-errpath-fixes
Open

agenticode wants to merge 2 commits into
mchehab:masterfrom
agenticode:x86-errpath-fixes

Conversation

@agenticode

Copy link
Copy Markdown

Two x86 error-path fixes, one commit each.

mce-amd-smca: bank_type used uninitialized

decode_smca_error() only sets bank_type when the IPID matches an entry in smca_hwid_mcatypes[]. The no-match check is i >= MAX_NR_BANKS (64), but the loop stops at ARRAY_SIZE(smca_hwid_mcatypes) (37), so it is never true and stack garbage indexes smca_names[] and smca_mce_descs[].

One mce_record with family 0x17 and IPID 0xffff0fff00000000 (in no entry, not a non-CPU node), under valgrind:

Use of uninitialised value of size 8
   at decode_smca_error (mce-amd-smca.c:964)    <- smca_names[bank_type].name
Use of uninitialised value of size 8
   at decode_smca_error (mce-amd-smca.c:969)    <- smca_mce_descs[bank_type].num_descs

Outside valgrind the same record printed "Don't know how to decode this bank", so the result depends on what is on the stack. With the patch bank_type starts at N_SMCA_BANK_TYPES and that message is what you always get.

I left the i >= MAX_NR_BANKS test alone: aecf33a moved it off ARRAY_SIZE() so that non-CPU nodes with no table match keep SMCA_UMC_V2.

ras-erst: mce_priv leak

ras_erst_init() calls init_mce_priv(), but x86-mce-erst has no .cleanup. The only free_mce_priv() caller is the x86-mce-event cleanup, and modules_cleanup_type() skips it when that module failed to init.

Failing the allocation x86-mce-event needs:

rasdaemon: module x86-mce-event init failed: -12
==28371==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 96 byte(s) in 1 object(s) allocated from:
    #2 init_mce_priv ../events-arch-x86/ras-mce-handler.c:401
    #3 ras_erst_init ../events-arch-x86/ras-erst.c:230

Indirect leak of 560 byte(s) in 1 object(s) allocated from:
    #1 __GI___getdelim

free_mce_priv() clears ras->mce_priv, so both modules calling it is fine.

Testing

Fedora 44, on top of d06e494:

  • gcc 16.2 and clang 22.1: no build warnings
  • meson test: 9/9 with both compilers
  • python3 tests/run.py: 140 tests, rc=0
  • scripts/checkpatch.pl --no-tree --strict: no issues

decode_smca_error() leaves bank_type untouched when the IPID matches no
entry in smca_hwid_mcatypes[]. The test meant to catch that is
"i >= MAX_NR_BANKS" (64), but the loop only runs to
ARRAY_SIZE(smca_hwid_mcatypes), which is 37, so it can never be true.
The uninitialized value then indexes smca_names[] and smca_mce_descs[].

An mce_record with an IPID that is not in the table decodes differently
on every run; valgrind reports the uninitialized reads.

Set bank_type to N_SMCA_BANK_TYPES up front so that case falls into the
"Don't know how to decode this bank" check just below. The
"i >= MAX_NR_BANKS" test stays: aecf33a ("rasdaemon: Enumerate memory on
noncpu nodes") moved it off ARRAY_SIZE() so that non-CPU nodes without a
table match keep SMCA_UMC_V2.

Signed-off-by: Jongwon Lee <ai@linux.com>
ras_erst_init() allocates ras->mce_priv through init_mce_priv(), but
x86-mce-erst registers no cleanup. The only free_mce_priv() caller is
x86-mce-event's cleanup, and modules_cleanup_type() skips modules whose
is_enabled is false, so nothing frees it if that module failed to
initialize.

Add the cleanup. free_mce_priv() clears ras->mce_priv, so having both
modules call it is fine.

With the allocation x86-mce-event needs made to fail, LeakSanitizer
reports 96 bytes direct plus 560 indirect.

Signed-off-by: Jongwon Lee <ai@linux.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant