2006-11-29 22:08:09

by Chris Wright

[permalink] [raw]
Subject: [patch 14/23] x86 microcode: dont check the size

-stable review patch. If anyone has any objections, please let us know.
------------------

From: Shaohua Li <[email protected]>

IA32 manual says if micorcode update's size is 0, then the size is
default size (2048 bytes). But this doesn't suggest all microcode
update's size should be above 2048 bytes to me. We actually had a
microcode update whose size is 1024 bytes. The patch just removed the
check.

Backported to 2.6.18 by Daniel Drake.

Signed-off-by: Daniel Drake <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
---
arch/i386/kernel/microcode.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)

--- linux-2.6.18.4.orig/arch/i386/kernel/microcode.c
+++ linux-2.6.18.4/arch/i386/kernel/microcode.c
@@ -250,14 +250,14 @@ static int find_matching_ucodes (void)
}

total_size = get_totalsize(&mc_header);
- if ((cursor + total_size > user_buffer_size) || (total_size < DEFAULT_UCODE_TOTALSIZE)) {
+ if (cursor + total_size > user_buffer_size) {
printk(KERN_ERR "microcode: error! Bad data in microcode data file\n");
error = -EINVAL;
goto out;
}

data_size = get_datasize(&mc_header);
- if ((data_size + MC_HEADER_SIZE > total_size) || (data_size < DEFAULT_UCODE_DATASIZE)) {
+ if (data_size + MC_HEADER_SIZE > total_size) {
printk(KERN_ERR "microcode: error! Bad data in microcode data file\n");
error = -EINVAL;
goto out;
@@ -460,11 +460,6 @@ static ssize_t microcode_write (struct f
{
ssize_t ret;

- if (len < DEFAULT_UCODE_TOTALSIZE) {
- printk(KERN_ERR "microcode: not enough data\n");
- return -EINVAL;
- }
-
if ((len >> PAGE_SHIFT) > num_physpages) {
printk(KERN_ERR "microcode: too much data (max %ld pages)\n", num_physpages);
return -EINVAL;

--


2006-12-02 06:45:06

by Willy Tarreau

[permalink] [raw]
Subject: Re: [patch 14/23] x86 microcode: dont check the size

Shaohua,

this one seems appropriate for 2.4 too. It is OK for you if I merge it ?

Thanks,
Willy

On Wed, Nov 29, 2006 at 02:00:25PM -0800, Chris Wright wrote:
> -stable review patch. If anyone has any objections, please let us know.
> ------------------
>
> From: Shaohua Li <[email protected]>
>
> IA32 manual says if micorcode update's size is 0, then the size is
> default size (2048 bytes). But this doesn't suggest all microcode
> update's size should be above 2048 bytes to me. We actually had a
> microcode update whose size is 1024 bytes. The patch just removed the
> check.
>
> Backported to 2.6.18 by Daniel Drake.
>
> Signed-off-by: Daniel Drake <[email protected]>
> Signed-off-by: Chris Wright <[email protected]>
> ---
> arch/i386/kernel/microcode.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> --- linux-2.6.18.4.orig/arch/i386/kernel/microcode.c
> +++ linux-2.6.18.4/arch/i386/kernel/microcode.c
> @@ -250,14 +250,14 @@ static int find_matching_ucodes (void)
> }
>
> total_size = get_totalsize(&mc_header);
> - if ((cursor + total_size > user_buffer_size) || (total_size < DEFAULT_UCODE_TOTALSIZE)) {
> + if (cursor + total_size > user_buffer_size) {
> printk(KERN_ERR "microcode: error! Bad data in microcode data file\n");
> error = -EINVAL;
> goto out;
> }
>
> data_size = get_datasize(&mc_header);
> - if ((data_size + MC_HEADER_SIZE > total_size) || (data_size < DEFAULT_UCODE_DATASIZE)) {
> + if (data_size + MC_HEADER_SIZE > total_size) {
> printk(KERN_ERR "microcode: error! Bad data in microcode data file\n");
> error = -EINVAL;
> goto out;
> @@ -460,11 +460,6 @@ static ssize_t microcode_write (struct f
> {
> ssize_t ret;
>
> - if (len < DEFAULT_UCODE_TOTALSIZE) {
> - printk(KERN_ERR "microcode: not enough data\n");
> - return -EINVAL;
> - }
> -
> if ((len >> PAGE_SHIFT) > num_physpages) {
> printk(KERN_ERR "microcode: too much data (max %ld pages)\n", num_physpages);
> return -EINVAL;
>
> --
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2006-12-03 01:25:52

by Bill Davidsen

[permalink] [raw]
Subject: Re: [patch 14/23] x86 microcode: dont check the size

Willy Tarreau wrote:
> Shaohua,
>
> this one seems appropriate for 2.4 too. It is OK for you if I merge it ?
>
> Thanks,
> Willy
>
> On Wed, Nov 29, 2006 at 02:00:25PM -0800, Chris Wright wrote:
>> -stable review patch. If anyone has any objections, please let us know.
>> ------------------
>>
>> From: Shaohua Li <[email protected]>
>>
>> IA32 manual says if micorcode update's size is 0, then the size is
>> default size (2048 bytes). But this doesn't suggest all microcode
>> update's size should be above 2048 bytes to me. We actually had a
>> microcode update whose size is 1024 bytes. The patch just removed the
>> check.

I agree with what you said, but unless I miss something, not what you
did... I don't see the code to get the size and set it to 2k if it's
zero. I would expect after the call to get_totalsize() that there would
be a line like:
if (unlikely(total_size == 0)) total_size = DEFAULT_UCODE_TOTALSIZE;
or some similar logic to do what the manual suggests, that zero is a
valid value.

I may be totally misreading this, of course, I'm taking the manual quote
as gospel.

--
Bill Davidsen <[email protected]>
"We have more to fear from the bungling of the incompetent than from
the machinations of the wicked." - from Slashdot

2006-12-04 01:04:46

by Shaohua Li

[permalink] [raw]
Subject: Re: [patch 14/23] x86 microcode: dont check the size

On Sat, 2006-12-02 at 07:44 +0100, Willy Tarreau wrote:
> Shaohua,
>
> this one seems appropriate for 2.4 too. It is OK for you if I merge it ?
Yes, 2.4 and 2.6 use the same driver. It should be fine to merge it.

Thanks,
Shaohua

> On Wed, Nov 29, 2006 at 02:00:25PM -0800, Chris Wright wrote:
> > -stable review patch. If anyone has any objections, please let us know.
> > ------------------
> >
> > From: Shaohua Li <[email protected]>
> >
> > IA32 manual says if micorcode update's size is 0, then the size is
> > default size (2048 bytes). But this doesn't suggest all microcode
> > update's size should be above 2048 bytes to me. We actually had a
> > microcode update whose size is 1024 bytes. The patch just removed the
> > check.
> >
> > Backported to 2.6.18 by Daniel Drake.
> >
> > Signed-off-by: Daniel Drake <[email protected]>
> > Signed-off-by: Chris Wright <[email protected]>
> > ---
> > arch/i386/kernel/microcode.c | 9 ++-------
> > 1 file changed, 2 insertions(+), 7 deletions(-)
> >
> > --- linux-2.6.18.4.orig/arch/i386/kernel/microcode.c
> > +++ linux-2.6.18.4/arch/i386/kernel/microcode.c
> > @@ -250,14 +250,14 @@ static int find_matching_ucodes (void)
> > }
> >
> > total_size = get_totalsize(&mc_header);
> > - if ((cursor + total_size > user_buffer_size) || (total_size < DEFAULT_UCODE_TOTALSIZE)) {
> > + if (cursor + total_size > user_buffer_size) {
> > printk(KERN_ERR "microcode: error! Bad data in microcode data file\n");
> > error = -EINVAL;
> > goto out;
> > }
> >
> > data_size = get_datasize(&mc_header);
> > - if ((data_size + MC_HEADER_SIZE > total_size) || (data_size < DEFAULT_UCODE_DATASIZE)) {
> > + if (data_size + MC_HEADER_SIZE > total_size) {
> > printk(KERN_ERR "microcode: error! Bad data in microcode data file\n");
> > error = -EINVAL;
> > goto out;
> > @@ -460,11 +460,6 @@ static ssize_t microcode_write (struct f
> > {
> > ssize_t ret;
> >
> > - if (len < DEFAULT_UCODE_TOTALSIZE) {
> > - printk(KERN_ERR "microcode: not enough data\n");
> > - return -EINVAL;
> > - }
> > -
> > if ((len >> PAGE_SHIFT) > num_physpages) {
> > printk(KERN_ERR "microcode: too much data (max %ld pages)\n", num_physpages);
> > return -EINVAL;
> >
> > --
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/

2006-12-04 01:31:28

by Willy Tarreau

[permalink] [raw]
Subject: Re: [patch 14/23] x86 microcode: dont check the size

On Mon, Dec 04, 2006 at 09:04:03AM +0800, Shaohua Li wrote:
> On Sat, 2006-12-02 at 07:44 +0100, Willy Tarreau wrote:
> > Shaohua,
> >
> > this one seems appropriate for 2.4 too. It is OK for you if I merge it ?
> Yes, 2.4 and 2.6 use the same driver. It should be fine to merge it.
>
> Thanks,
> Shaohua

Queued, thank you !
Willy