Skip to contentSkip to conversation

disk: part_efi: fix regression due to incorrect buffer usage

PATCH New Stephen Warren 7 participants 12 messages Oct 28, 2011 – Nov 23, 2011

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 disk: part_efi: fix regression due to incorrect buffer usage

Commit deb5ca80275e8cfa74d5680b41204e08a095eca5 "disk: part_efi: fix **pgpt_pte == NULL" modified the code to pass "&gpt_head" to is_gpt_valid() rather than the previous "gpt_head". However, gpt_head is a pointer to the buffer, not the actual buffer, since it was allocated using ALLOC_CACHE_ALIGN_BUFFER. This caused is_gpt_valid() to read the disk block onto the stack rather than into the buffer, causing the code to fail.

This change reverts that portion of the commit mentioned above.

---
 disk/part_efi.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
disk/part_efi.c +2 -2
index e7f2714..ddf80a7 100644
@@ -130,7 +130,7 @@ void print_part_efi(block_dev_desc_t * dev_desc)
 	}
 	/* This function validates AND fills in the GPT header and PTE */
 	if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
-			 &(gpt_head), &gpt_pte) != 1) {
+			 gpt_head, &gpt_pte) != 1) {
 		printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
 		return;
 	}
@@ -169,7 +169,7 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part,
 
 	/* This function validates AND fills in the GPT header and PTE */
 	if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
-			&(gpt_head), &gpt_pte) != 1) {
+			gpt_head, &gpt_pte) != 1) {
 		printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
 		return -1;
 	}
Signature · 1 line
1.7.0.4
#1 lore raw

PATCH 1/1 disk: part_efi: fix regression due to incorrect buffer usage

...to be fair though, the regression appears to be caused by a mid-air collision of Anton's change (f75dd584cdfe29dfdcfd424bb237b9238cfb8fe4) with my change. His patch was committed on Oct 25th (though authored earlier).
When I submitted my patch on the Oct 19th against denx/master, I'm pretty
sure it was correct.

Thank you for finding / fixing! :)

-Doug

---

Quoted 42 lines · Stephen Warren

Commit deb5ca80275e8cfa74d5680b41204e08a095eca5 "disk: part_efi: fix **pgpt_pte == NULL" modified the code to pass "&gpt_head" to is_gpt_valid() rather than the previous "gpt_head". However, gpt_head is a pointer to the buffer, not the actual buffer, since it was allocated using ALLOC_CACHE_ALIGN_BUFFER. This caused is_gpt_valid() to read the disk block onto the stack rather than into the buffer, causing the code to fail.

This change reverts that portion of the commit mentioned above.

---
 disk/part_efi.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
disk/part_efi.c +2 -2
index e7f2714..ddf80a7 100644
@@ -130,7 +130,7 @@ void print_part_efi(block_dev_desc_t * dev_desc)
       }
       /* This function validates AND fills in the GPT header and PTE */
       if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
-                        &(gpt_head), &gpt_pte) != 1) {
+                        gpt_head, &gpt_pte) != 1) {
               printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
               return;
       }
@@ -169,7 +169,7 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc,
 int part,
 
       /* This function validates AND fills in the GPT header and PTE */
       if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
-                       &(gpt_head), &gpt_pte) != 1) {
+                       gpt_head, &gpt_pte) != 1) {
               printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
               return -1;
       }

--
1.7.0.4

#2 lore raw

PATCH 1/1 disk: part_efi: fix regression due to incorrect buffer usage

Thanks Stephen, sorry Doug. :)

-Anton

Quoted 65 lines · Doug Anderson

...to be fair though, the regression appears to be caused by a mid-air collision of Anton's change (f75dd584cdfe29dfdcfd424bb237b9238cfb8fe4) with my change. ?His patch was committed on Oct 25th (though authored earlier). ?When I submitted my patch on the Oct 19th against denx/master, I'm pretty sure it was correct.

Thank you for finding / fixing! ?:)

-Doug

---

Quoted 32 lines · Stephen Warren

Commit deb5ca80275e8cfa74d5680b41204e08a095eca5 "disk: part_efi: fix **pgpt_pte == NULL" modified the code to pass "&gpt_head" to is_gpt_valid() rather than the previous "gpt_head". However, gpt_head is a pointer to the buffer, not the actual buffer, since it was allocated using ALLOC_CACHE_ALIGN_BUFFER. This caused is_gpt_valid() to read the disk block onto the stack rather than into the buffer, causing the code to fail.

This change reverts that portion of the commit mentioned above.

---
?disk/part_efi.c | ? ?4 ++--
?1 files changed, 2 insertions(+), 2 deletions(-)

disk/part_efi.c +0 -0
index e7f2714..ddf80a7 100644
@@ -130,7 +130,7 @@ void print_part_efi(block_dev_desc_t * dev_desc)
 ? ? ? ?}

? ? ? ?/* This function validates AND fills in the GPT header and PTE */ ? ? ? ?if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
- ? ? ? ? ? ? ? ? ? ? ? ?&(gpt_head), &gpt_pte) != 1) {
+ ? ? ? ? ? ? ? ? ? ? ? ?gpt_head, &gpt_pte) != 1) {
? ? ? ? ? ? ? ?printf("%s: *** ERROR: Invalid GPT ***\n", __func__); ? ? ? ? ? ? ? ?return;
? ? ? ?}

disk/part_efi.c +0 -0
@@ -169,7 +169,7 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc,
 int part,
 

? ? ? ?/* This function validates AND fills in the GPT header and PTE */

Quoted 10 lines

? ? ? ?if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
- ? ? ? ? ? ? ? ? ? ? ? &(gpt_head), &gpt_pte) != 1) {
+ ? ? ? ? ? ? ? ? ? ? ? gpt_head, &gpt_pte) != 1) {
? ? ? ? ? ? ? ?printf("%s: *** ERROR: Invalid GPT ***\n", __func__); ? ? ? ? ? ? ? ?return -1;
? ? ? ?}
--
1.7.0.4

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#3 lore raw

PATCH 1/1 disk: part_efi: fix regression due to incorrect buffer usage

Quoted 6 lines · Anton Staaf

Thanks Stephen, sorry Doug. ?:)

-Anton

Quoted 1 line · Doug Anderson
Quoted 69 lines
Quoted 46 lines

...to be fair though, the regression appears to be caused by a mid-air collision of Anton's change (f75dd584cdfe29dfdcfd424bb237b9238cfb8fe4) with my change. ?His patch was committed on Oct 25th (though authored earlier). ?When I submitted my patch on the Oct 19th against denx/master, I'm pretty sure it was correct.

Thank you for finding / fixing! ?:)

-Doug

---

Quoted 31 lines · Stephen Warren

Commit deb5ca80275e8cfa74d5680b41204e08a095eca5 "disk: part_efi: fix **pgpt_pte == NULL" modified the code to pass "&gpt_head" to is_gpt_valid() rather than the previous "gpt_head". However, gpt_head is a pointer to the buffer, not the actual buffer, since it was allocated using ALLOC_CACHE_ALIGN_BUFFER. This caused is_gpt_valid() to read the disk block onto the stack rather than into the buffer, causing the code to fail.

This change reverts that portion of the commit mentioned above.

---
?disk/part_efi.c | ? ?4 ++--
?1 files changed, 2 insertions(+), 2 deletions(-)

disk/part_efi.c +0 -0
index e7f2714..ddf80a7 100644
@@ -130,7 +130,7 @@ void print_part_efi(block_dev_desc_t * dev_desc)
 ? ? ? ?}

? ? ? ?/* This function validates AND fills in the GPT header and PTE */ ? ? ? ?if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
- ? ? ? ? ? ? ? ? ? ? ? ?&(gpt_head), &gpt_pte) != 1) {
+ ? ? ? ? ? ? ? ? ? ? ? ?gpt_head, &gpt_pte) != 1) {
? ? ? ? ? ? ? ?printf("%s: *** ERROR: Invalid GPT ***\n", __func__); ? ? ? ? ? ? ? ?return;
? ? ? ?}

disk/part_efi.c +0 -0
@@ -169,7 +169,7 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc,
 int part,
 
Quoted 18 lines
Quoted 11 lines

? ? ? ?/* This function validates AND fills in the GPT header and PTE */ ? ? ? ?if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
- ? ? ? ? ? ? ? ? ? ? ? &(gpt_head), &gpt_pte) != 1) {
+ ? ? ? ? ? ? ? ? ? ? ? gpt_head, &gpt_pte) != 1) {
? ? ? ? ? ? ? ?printf("%s: *** ERROR: Invalid GPT ***\n", __func__); ? ? ? ? ? ? ? ?return -1;
? ? ? ?}
--
1.7.0.4

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#6 lore raw

PATCH 1/1 disk: part_efi: fix regression due to incorrect buffer usage

Quoted 7 lines · Anton Staaf

Mike, do you think you could pull this into staging as per Wolfgang's recent request for help?

Thanks,
? ?Anton

Please can one of the maintainers grab this and put it in staging?

Regards,
Simon

Quoted 15 lines · Mike Frysinger
Quoted 9 lines

-mike

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#7 lore raw

PATCH 1/1 disk: part_efi: fix regression due to incorrect buffer usage

Hi Simon,

Quoted 10 lines · On Tue, 22 Nov 2011 14:10:12 -0800 Simon Glass <[email protected]> wrote:
Quoted 7 lines · Anton Staaf

Mike, do you think you could pull this into staging as per Wolfgang's recent request for help?

Thanks,
? ?Anton

Please can one of the maintainers grab this and put it in staging?

I've seen similar patch, it is already in arm tree:

http://git.denx.de/?p=u-boot/u-boot-arm.git;a=commit;h=81432275812e7d9b3b5eec9c54c6c58dfe506759

Anatolij

#8 lore raw

PATCH 1/1 disk: part_efi: fix regression due to incorrect buffer usage

Quoted 14 lines · Anatolij Gustschin

Hi Simon,

Quoted 9 lines · On Tue, 22 Nov 2011 14:10:12 -0800 Simon Glass <[email protected]> wrote:
Quoted 7 lines · Anton Staaf

Mike, do you think you could pull this into staging as per Wolfgang's recent request for help?

Thanks,
? ?Anton

Quoted 5 lines
Quoted 1 line

Please can one of the maintainers grab this and put it in staging?

I've seen similar patch, it is already in arm tree:

http://git.denx.de/?p=u-boot/u-boot-arm.git;a=commit;h=81432275812e7d9b3b5eec9c54c6c58dfe506759

OK, that's a funny place for it, but OK. Thanks for pointing that out.

Regards,
Simon

Quoted 3 lines

Anatolij

#9 lore raw

PATCH 1/1 disk: part_efi: fix regression due to incorrect buffer usage

In reply to Simon Glass
Quoted 10 lines · Simon Glass
Quoted 7 lines · Anton Staaf

Mike, do you think you could pull this into staging as per Wolfgang's recent request for help?

Thanks,
Anton

Please can one of the maintainers grab this and put it in staging?

Hi Simon,

I grab it in my TODO list in patchwork, I will take a look at it.

Stefano

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#10 lore raw

PATCH 1/1 disk: part_efi: fix regression due to incorrect buffer usage

Quoted 10 lines · Stefano Babic
Quoted 9 lines · Simon Glass
Quoted 7 lines · Anton Staaf

Mike, do you think you could pull this into staging as per Wolfgang's recent request for help?

Thanks,
? ?Anton

Quoted 5 lines
Quoted 1 line

Please can one of the maintainers grab this and put it in staging?

Hi Simon,

I grab it in my TODO list in patchwork, I will take a look at it.

Thanks Stefano - please also see Anatolij's email on this thread. It seems that Albert may have picked up a similar patch.

Regards,
Simon

Quoted 10 lines

Stefano

--
=====================================================================
DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 ?Email: office at denx.de
=====================================================================

#11 lore raw

PATCH 1/1 disk: part_efi: fix regression due to incorrect buffer usage

Am 23/11/2011 20:18, schrieb Simon Glass:

Quoted 3 lines

Thanks Stefano - please also see Anatolij's email on this thread. It seems that Albert may have picked up a similar patch.

In fact the patch "Fix errors noticed after enabling CONFIG_EFI_PARTITION for the OMAP3 EVM board" has already fixed this issue. There is no need to merge this patch.

Best regards.
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================