Fix EDD3.0 data verification.
PATCH v2 1/1 Fix EDD3.0 data verification.
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.
---v1->v2
use device path info length to calculate csum.
drivers/firmware/edd.c +11 -11
@@ -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.
Peter, ping.
Is this version acceptable?
Quoted 58 lines · Gleb Natapov
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 theinfo->paramsstructure 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 toedd_infostructure (commit66b61a5cin 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 <[email protected]>---
v1->v2
use device path info length to calculate csum.drivers/firmware/edd.c +11
-12@@ -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 ; } --
Quoted 6 lines
Gleb.
--
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/
--
Gleb.
Quoted 4 lines · Gleb Natapov
Peter, ping.
Is this version acceptable?
Ping?
Quoted 66 lines · Gleb Natapov
Quoted 58 lines
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 theinfo->paramsstructure 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 toedd_infostructure (commit66b61a5cin 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 <[email protected]>---
v1->v2
use device path info length to calculate csum.drivers/firmware/edd.c +11
-12@@ -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 ; } --Quoted 6 lines
Gleb.
--
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/
Quoted 7 lines
--
Gleb.
--
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/
--
Gleb.
Quoted 7 lines · Gleb Natapov
Quoted 4 lines · Gleb Natapov
Peter, ping.
Is this version acceptable?
Ping?
Yes, looks good... I'll try to pull it into the tree today.
-hpa
Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf.
Quoted 11 lines · H. Peter Anvin
Quoted 7 lines · Gleb Natapov
Quoted 4 lines · Gleb Natapov
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.
Quoted 7 lines · Gleb Natapov
Quoted 6 lines · H. Peter Anvin
Quoted 5 lines · Gleb Natapov
Quoted 4 lines · Gleb Natapov
Peter, ping.
Is this version acceptable?
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
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.