Bug 179523 - [cpuctl] [patch] Memory leak/corruption in cpuctl pseudo-driver and more
Summary: [cpuctl] [patch] Memory leak/corruption in cpuctl pseudo-driver and more
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 10.1-RELEASE
Hardware: Any Any
: Normal Affects Some People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-06-12 21:40 UTC by John Clark
Modified: 2019-01-20 03:51 UTC (History)
5 users (show)

See Also:


Attachments
file.diff (2.00 KB, patch)
2013-06-12 21:40 UTC, John Clark
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Clark 2013-06-12 21:40:00 UTC
1) Processor microcode update functions update_intel and update_via in sys/dev/cpuctl/cpuctl.c allocate a buffer with malloc but free it with contigfree.  update_amd appears to be correct because it does contigmalloc followed by contigfree.

2) Also, the UCODE_SIZE_MAX limit of 10 KB is too small for some microcode updates.  I noticed this because I tried to apply the most recent update for an Intel Core i5-3570, which is 16 KB, and it failed due to this limit.  Currently, the largest microcode update I can find is 28 KB, so I raised the limit to 32 KB to accommodate this.

I have fixed and tested both issues and attached a patch.  The patch applies cleanly both to the head and to 9.1-RELEASE-p3.

Fix: Simply apply the attached patch and rebuild.

Patch attached with submission follows:
How-To-Repeat: 1) Found by inspection of sys/dev/cpuctl/cpuctl.c
2) Try to apply a microcode update to an Intel, AMD, or VIA processor that exceeds 10 KB in size.
Comment 1 John Clark 2013-06-12 21:46:57 UTC
P.S.  Oops, it looks like I reversed my name and email address in the
web submission form.  I should have set up sendmail and used send-pr...

- John
Comment 2 Vladimir Laskov 2014-12-19 22:56:39 UTC
# uname -a
FreeBSD anonymous.orb 10.1-RELEASE FreeBSD 10.1-RELEASE #0 r274401: Tue Nov 11 21:02:49 UTC 2014     root@releng1.nyi.freebsd.org:/usr/obj/usr/src/sys/GENERIC  amd64
# dmesg | grep ^CPU
CPU: Intel(R) Xeon(R) CPU E3-1270 v3 @ 3.50GHz (3491.99-MHz K8-class CPU)


# /usr/sbin/cpucontrol -u -d /usr/local/share/cpucontrol /dev/cpuctl1
/usr/local/share/cpucontrol/m32306c3_0000001c.fw: updating cpu /dev/cpuctl1 from rev 0x9 to rev 0x1c... failed.
cpucontrol: ioctl(): Invalid argument
/usr/local/share/cpucontrol/06-3c-03: updating cpu /dev/cpuctl1 from rev 0x9 to rev 0x16... failed.
cpucontrol: ioctl(): Invalid argument
/usr/local/share/cpucontrol/m32306c3_0000001c.fw: updating cpu /dev/cpuctl1 from rev 0x9 to rev 0x1c... failed.
cpucontrol: ioctl(): Invalid argument
/usr/local/share/cpucontrol/06-3c-03: updating cpu /dev/cpuctl1 from rev 0x9 to rev 0x16... failed.
cpucontrol: ioctl(): Invalid argument


# grep "UCODE_SIZE_MAX" /usr/src/sys/dev/cpuctl/cpuctl.c | head -1
#define	UCODE_SIZE_MAX	(16 * 1024)

# patch  -p0 < file.patch 
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|--- sys/dev/cpuctl/cpuctl.c.orig	2012-12-03 19:52:24.000000000 -0500
|+++ sys/dev/cpuctl/cpuctl.c	2013-06-12 13:15:20.000000000 -0400
--------------------------
Patching file sys/dev/cpuctl/cpuctl.c using Plan A...
Hunk #1 failed at 63.
Hunk #2 failed at 295.
Hunk #3 failed at 314.
Hunk #4 succeeded at 416 with fuzz 2 (offset 70 lines).
Hunk #5 failed at 479.
Hunk #6 failed at 498.
Hunk #7 failed at 546.
6 out of 7 hunks failed--saving rejects to sys/dev/cpuctl/cpuctl.c.rej
done


John, please modify patch for 10.1-RELEASE
Comment 3 commit-hook freebsd_committer freebsd_triage 2014-12-20 16:41:12 UTC
A commit references this bug:

Author: kib
Date: Sat Dec 20 16:40:50 UTC 2014
New revision: 275960
URL: https://svnweb.freebsd.org/changeset/base/275960

Log:
  Increase allowed size of the microcode blob to 32KB.  Some Intel CPU's
  updates weight 28KB.

  PR:	179523
  MFC after:	1 week

Changes:
  head/sys/dev/cpuctl/cpuctl.c
Comment 4 Glen Barber freebsd_committer freebsd_triage 2015-07-08 18:32:02 UTC
To originators/assignees of this PR:

A commit to the tree references this PR, however the PR is still in a non-closed state.

Please review this PR and close as appropriate, or if closing the PR requires a merge to stable/10, please let re@ know as soon as possible.

Thank you.

Glen
Comment 5 Stefan B. 2018-01-13 08:24:19 UTC
The current version of cpuctl.c contains this definition

#define	UCODE_SIZE_MAX	(4 * 1024 * 1024)

which should be sufficent for some years, as the latest Intel microcode release's maximum filesize is close to 100kB.

So I think this PR can be closed.