LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCHv2] Fix EDD3.0 data verification.
@ 2011-02-03 12:29 Gleb Natapov
2011-02-08 14:53 ` Gleb Natapov
0 siblings, 1 reply; 6+ messages in thread
From: Gleb Natapov @ 2011-02-03 12:29 UTC (permalink / raw)
To: linux-kernel; +Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86
Check for nonzero path in edd_has_edd30() has no sense. First, it looks
at the wrong memory. Device path starts at offset 30 of the info->params
structure which is at offset 8 from the beginning of info structure, but
code looks at info + 4 instead. This was correct when code was introduced,
but around v2.6.4 three more fields were added to edd_info structure
(commit 66b61a5c in history.git). Second, even if it will check correct
memory it will always succeed since at offset 30 (params->key) there will
be non-zero values otherwise previous check would fail.
The patch replaces this bogus check with one that verifies checksum.
Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
v1->v2
use device path info length to calculate csum.
diff --git a/drivers/firmware/edd.c b/drivers/firmware/edd.c
index 96c25d9..f1b7f65 100644
--- a/drivers/firmware/edd.c
+++ b/drivers/firmware/edd.c
@@ -531,8 +531,8 @@ static int
edd_has_edd30(struct edd_device *edev)
{
struct edd_info *info;
- int i, nonzero_path = 0;
- char c;
+ int i;
+ u8 csum = 0;
if (!edev)
return 0;
@@ -544,16 +544,16 @@ edd_has_edd30(struct edd_device *edev)
return 0;
}
- for (i = 30; i <= 73; i++) {
- c = *(((uint8_t *) info) + i + 4);
- if (c) {
- nonzero_path++;
- break;
- }
- }
- if (!nonzero_path) {
+
+ /* We support only T13 spec */
+ if (info->params.device_path_info_length != 44)
+ return 0;
+
+ for (i = 30; i < info->params.device_path_info_length + 30; i++)
+ csum += *(((u8 *)&info->params) + i);
+
+ if (csum)
return 0;
- }
return 1;
}
--
Gleb.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv2] Fix EDD3.0 data verification.
2011-02-03 12:29 [PATCHv2] Fix EDD3.0 data verification Gleb Natapov
@ 2011-02-08 14:53 ` Gleb Natapov
2011-02-16 15:09 ` Gleb Natapov
0 siblings, 1 reply; 6+ messages in thread
From: Gleb Natapov @ 2011-02-08 14:53 UTC (permalink / raw)
To: linux-kernel; +Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86
Peter, ping.
Is this version acceptable?
On Thu, Feb 03, 2011 at 02:29:48PM +0200, Gleb Natapov wrote:
> Check for nonzero path in edd_has_edd30() has no sense. First, it looks
> at the wrong memory. Device path starts at offset 30 of the info->params
> structure which is at offset 8 from the beginning of info structure, but
> code looks at info + 4 instead. This was correct when code was introduced,
> but around v2.6.4 three more fields were added to edd_info structure
> (commit 66b61a5c in history.git). Second, even if it will check correct
> memory it will always succeed since at offset 30 (params->key) there will
> be non-zero values otherwise previous check would fail.
>
> The patch replaces this bogus check with one that verifies checksum.
>
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> ---
> v1->v2
> use device path info length to calculate csum.
>
> diff --git a/drivers/firmware/edd.c b/drivers/firmware/edd.c
> index 96c25d9..f1b7f65 100644
> --- a/drivers/firmware/edd.c
> +++ b/drivers/firmware/edd.c
> @@ -531,8 +531,8 @@ static int
> edd_has_edd30(struct edd_device *edev)
> {
> struct edd_info *info;
> - int i, nonzero_path = 0;
> - char c;
> + int i;
> + u8 csum = 0;
>
> if (!edev)
> return 0;
> @@ -544,16 +544,16 @@ edd_has_edd30(struct edd_device *edev)
> return 0;
> }
>
> - for (i = 30; i <= 73; i++) {
> - c = *(((uint8_t *) info) + i + 4);
> - if (c) {
> - nonzero_path++;
> - break;
> - }
> - }
> - if (!nonzero_path) {
> +
> + /* We support only T13 spec */
> + if (info->params.device_path_info_length != 44)
> + return 0;
> +
> + for (i = 30; i < info->params.device_path_info_length + 30; i++)
> + csum += *(((u8 *)&info->params) + i);
> +
> + if (csum)
> return 0;
> - }
>
> return 1;
> }
> --
> Gleb.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Gleb.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv2] Fix EDD3.0 data verification.
2011-02-08 14:53 ` Gleb Natapov
@ 2011-02-16 15:09 ` Gleb Natapov
2011-02-16 18:57 ` H. Peter Anvin
0 siblings, 1 reply; 6+ messages in thread
From: Gleb Natapov @ 2011-02-16 15:09 UTC (permalink / raw)
To: linux-kernel; +Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86
On Tue, Feb 08, 2011 at 04:53:01PM +0200, Gleb Natapov wrote:
> Peter, ping.
>
> Is this version acceptable?
>
Ping?
> On Thu, Feb 03, 2011 at 02:29:48PM +0200, Gleb Natapov wrote:
> > Check for nonzero path in edd_has_edd30() has no sense. First, it looks
> > at the wrong memory. Device path starts at offset 30 of the info->params
> > structure which is at offset 8 from the beginning of info structure, but
> > code looks at info + 4 instead. This was correct when code was introduced,
> > but around v2.6.4 three more fields were added to edd_info structure
> > (commit 66b61a5c in history.git). Second, even if it will check correct
> > memory it will always succeed since at offset 30 (params->key) there will
> > be non-zero values otherwise previous check would fail.
> >
> > The patch replaces this bogus check with one that verifies checksum.
> >
> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > ---
> > v1->v2
> > use device path info length to calculate csum.
> >
> > diff --git a/drivers/firmware/edd.c b/drivers/firmware/edd.c
> > index 96c25d9..f1b7f65 100644
> > --- a/drivers/firmware/edd.c
> > +++ b/drivers/firmware/edd.c
> > @@ -531,8 +531,8 @@ static int
> > edd_has_edd30(struct edd_device *edev)
> > {
> > struct edd_info *info;
> > - int i, nonzero_path = 0;
> > - char c;
> > + int i;
> > + u8 csum = 0;
> >
> > if (!edev)
> > return 0;
> > @@ -544,16 +544,16 @@ edd_has_edd30(struct edd_device *edev)
> > return 0;
> > }
> >
> > - for (i = 30; i <= 73; i++) {
> > - c = *(((uint8_t *) info) + i + 4);
> > - if (c) {
> > - nonzero_path++;
> > - break;
> > - }
> > - }
> > - if (!nonzero_path) {
> > +
> > + /* We support only T13 spec */
> > + if (info->params.device_path_info_length != 44)
> > + return 0;
> > +
> > + for (i = 30; i < info->params.device_path_info_length + 30; i++)
> > + csum += *(((u8 *)&info->params) + i);
> > +
> > + if (csum)
> > return 0;
> > - }
> >
> > return 1;
> > }
> > --
> > Gleb.
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
>
> --
> Gleb.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Gleb.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv2] Fix EDD3.0 data verification.
2011-02-16 15:09 ` Gleb Natapov
@ 2011-02-16 18:57 ` H. Peter Anvin
2011-03-16 8:57 ` Gleb Natapov
0 siblings, 1 reply; 6+ messages in thread
From: H. Peter Anvin @ 2011-02-16 18:57 UTC (permalink / raw)
To: Gleb Natapov; +Cc: linux-kernel, Thomas Gleixner, Ingo Molnar, x86
On 02/16/2011 07:09 AM, Gleb Natapov wrote:
> On Tue, Feb 08, 2011 at 04:53:01PM +0200, Gleb Natapov wrote:
>> Peter, ping.
>>
>> Is this version acceptable?
>>
> Ping?
>
Yes, looks good... I'll try to pull it into the tree today.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv2] Fix EDD3.0 data verification.
2011-02-16 18:57 ` H. Peter Anvin
@ 2011-03-16 8:57 ` Gleb Natapov
2011-04-05 15:41 ` Gleb Natapov
0 siblings, 1 reply; 6+ messages in thread
From: Gleb Natapov @ 2011-03-16 8:57 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: linux-kernel, Thomas Gleixner, Ingo Molnar, x86
On Wed, Feb 16, 2011 at 10:57:01AM -0800, H. Peter Anvin wrote:
> On 02/16/2011 07:09 AM, Gleb Natapov wrote:
> > On Tue, Feb 08, 2011 at 04:53:01PM +0200, Gleb Natapov wrote:
> >> Peter, ping.
> >>
> >> Is this version acceptable?
> >>
> > Ping?
> >
>
> Yes, looks good... I'll try to pull it into the tree today.
>
Hi Peter,
Was this applied? Will it go to 2.6.39?
--
Gleb.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv2] Fix EDD3.0 data verification.
2011-03-16 8:57 ` Gleb Natapov
@ 2011-04-05 15:41 ` Gleb Natapov
0 siblings, 0 replies; 6+ messages in thread
From: Gleb Natapov @ 2011-04-05 15:41 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: linux-kernel, Thomas Gleixner, Ingo Molnar, x86
On Wed, Mar 16, 2011 at 10:57:04AM +0200, Gleb Natapov wrote:
> On Wed, Feb 16, 2011 at 10:57:01AM -0800, H. Peter Anvin wrote:
> > On 02/16/2011 07:09 AM, Gleb Natapov wrote:
> > > On Tue, Feb 08, 2011 at 04:53:01PM +0200, Gleb Natapov wrote:
> > >> Peter, ping.
> > >>
> > >> Is this version acceptable?
> > >>
> > > Ping?
> > >
> >
> > Yes, looks good... I'll try to pull it into the tree today.
> >
> Hi Peter,
>
> Was this applied? Will it go to 2.6.39?
>
Ping. The patch is still not applied it seams.
--
Gleb.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-04-05 15:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-03 12:29 [PATCHv2] Fix EDD3.0 data verification Gleb Natapov
2011-02-08 14:53 ` Gleb Natapov
2011-02-16 15:09 ` Gleb Natapov
2011-02-16 18:57 ` H. Peter Anvin
2011-03-16 8:57 ` Gleb Natapov
2011-04-05 15:41 ` Gleb Natapov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).