On Wed, 2011-04-13 at 08:51 -0700, Greg KH wrote: > 2.6.32-longterm review patch. If anyone has any objections, please let us > know. > > ------------------ > > > From: Borislav Petkov <[email protected]> > > Upstream commit: 44d60c0f5c58c2168f31df9a481761451840eb54 > > The different families have a different max size for the ucode patch, > adjust size checking to the family we're running on. Also, do not > vzalloc the max size of the ucode but only the actual size that is > passed on from the firmware loader. [...] > @@ -125,6 +124,37 @@ static int get_matching_microcode(int cp > return 1; > } > > +static unsigned int verify_ucode_size(int cpu, const u8 *buf, unsigned int > size) > +{ > + struct cpuinfo_x86 *c = &cpu_data(cpu); > + unsigned int max_size, actual_size; > + > +#define F1XH_MPB_MAX_SIZE 2048 > +#define F14H_MPB_MAX_SIZE 1824 > +#define F15H_MPB_MAX_SIZE 4096 > + > + switch (c->x86) { > + case 0x14: > + max_size = F14H_MPB_MAX_SIZE; > + break; > + case 0x15: > + max_size = F15H_MPB_MAX_SIZE; > + break; > + default: > + max_size = F1XH_MPB_MAX_SIZE; > + break; > + } > + > + actual_size = buf[4] + (buf[5] << 8); > + > + if (actual_size > size || actual_size > max_size) {
Surely:
if (actual_size + UCODE_CONTAINER_SECTION_HDR > size || ...
> + pr_err("section size mismatch\n");
> + return 0;
> + }
> +
> + return actual_size;
> +}
> +
> static int apply_microcode_amd(int cpu)
> {
> u32 rev, dummy;
> @@ -164,11 +194,11 @@ static int get_ucode_data(void *to, cons
> }
>
> static void *
> -get_next_ucode(const u8 *buf, unsigned int size, unsigned int *mc_size)
> +get_next_ucode(int cpu, const u8 *buf, unsigned int size, unsigned int
> *mc_size)
> {
> - unsigned int total_size;
> + unsigned int actual_size = 0;
> u8 section_hdr[UCODE_CONTAINER_SECTION_HDR];
> - void *mc;
> + void *mc = NULL;
Dummy initialisations mean the compiler won't warn if you fail to
properly initialise them later.
> if (get_ucode_data(section_hdr, buf, UCODE_CONTAINER_SECTION_HDR))
> return NULL;
> @@ -179,23 +209,18 @@ get_next_ucode(const u8 *buf, unsigned i
> return NULL;
> }
>
> - total_size = (unsigned long) (section_hdr[4] + (section_hdr[5] << 8));
> + actual_size = verify_ucode_size(cpu, buf, size);
> + if (!actual_size)
> + return NULL;
>
> - if (total_size > size || total_size > UCODE_MAX_SIZE) {
> - printk(KERN_ERR "microcode: error: size mismatch\n");
> + mc = vmalloc(actual_size);
> + if (!mc)
> return NULL;
> - }
>
> - mc = vmalloc(UCODE_MAX_SIZE);
> - if (mc) {
> - memset(mc, 0, UCODE_MAX_SIZE);
> - if (get_ucode_data(mc, buf + UCODE_CONTAINER_SECTION_HDR,
> - total_size)) {
> - vfree(mc);
> - mc = NULL;
> - } else
> - *mc_size = total_size + UCODE_CONTAINER_SECTION_HDR;
> - }
> + memset(mc, 0, actual_size);
> + get_ucode_data(mc, buf + UCODE_CONTAINER_SECTION_HDR, actual_size);
[...]
So I wondered why the result of get_ucode_data() is no longer being
checked. And the answer is: because it's a trivial wrapper for
memcpy(), but with a 'return 0'. So the memset() is redundant.
Good thing nothing important depends on this validation, oh wait...
Ben.
--
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.
signature.asc
Description: This is a digitally signed message part
_______________________________________________ stable mailing list [email protected] http://linux.kernel.org/mailman/listinfo/stable
