Skip to contentSkip to conversation

arch/arm/lib/bootm: fix length check of kernel image

PATCH In review [email protected] 3 participants 4 messages Jul 21, 2014 – Jul 22, 2014 1 message not in the archive Arm (32-bit)

lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/1 arch/arm/lib/bootm: fix length check of kernel image

Signed-off-by Sebastian Block <[email protected]>
---
 arch/arm/lib/bootm.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/lib/bootm.c +1 -1
index 4896d01..d9b9111 100644
@@ -314,7 +314,7 @@ static int do_bootz_linux(struct image_data *data)
 			image_size - sizeof(*header));
 	if (ret < 0)
 		goto err_out;
-	if (ret < end - sizeof(*header)) {
+	if (ret < image_size - sizeof(*header)) {
 		printf("premature end of image\n");
 		ret = -EIO;
 		goto err_out;
Signature · 7 lines
1.7.10.4


_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
#1 lore raw

Hi Sebastian,

Quoted 10 lines · On Mon, Jul 21, 2014 at 12:43:13PM +0200, [email protected] wrote:
Signed-off-by Sebastian Block <[email protected]>
---
 arch/arm/lib/bootm.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/lib/bootm.c +0 -0
index 4896d01..d9b9111 100644

arch/arm/lib/bootm.c static int do_bootz_linux(struct image_data *data) · line 317

Quoted 5 lines
arch/arm/lib/bootm.c +1 -1
@@ -314,7 +314,7 @@ static int do_bootz_linux(struct image_data *data)
 			image_size - sizeof(*header));
 	if (ret < 0)
 		goto err_out;
-	if (ret < end - sizeof(*header)) {
+	if (ret < image_size - sizeof(*header)) {

Did this cause any real trouble? I was quite surprised when I saw that patch. After a test I saw that for me 'image_size' has the same value as 'end'.

The patch is obviously correct and will be applied, I'm just not sure if it's an important fix or a cleanup change.

Sascha

Signature · 9 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
#2 lore raw

Am Dienstag, den 22.07.2014, 07:38 +0200 schrieb Sascha Hauer:

Quoted 23 lines

Hi Sebastian,

Quoted 10 lines · On Mon, Jul 21, 2014 at 12:43:13PM +0200, [email protected] wrote:
Signed-off-by Sebastian Block <[email protected]>
---
 arch/arm/lib/bootm.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/lib/bootm.c +0 -0
index 4896d01..d9b9111 100644

arch/arm/lib/bootm.c static int do_bootz_linux(struct image_data *data) · line 317

Quoted 5 lines
arch/arm/lib/bootm.c +1 -1
@@ -314,7 +314,7 @@ static int do_bootz_linux(struct image_data *data)
 			image_size - sizeof(*header));
 	if (ret < 0)
 		goto err_out;
-	if (ret < end - sizeof(*header)) {
+	if (ret < image_size - sizeof(*header)) {

Did this cause any real trouble? I was quite surprised when I saw that patch. After a test I saw that for me 'image_size' has the same value as 'end'.

Quoted 3 lines

The patch is obviously correct and will be applied, I'm just not sure if it's an important fix or a cleanup change.

I don't think this qualifies as a critical fix, as before the introduction of this 'image_size' variable we always used 'end' as the size of the image without running into any problems. So I think this is only a correctness fix.

Regards,
Lucas

Signature · 8 lines
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |


_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

A message that is not in the archive (sent off-list or not yet received): [email protected]

#4 lore raw
Quoted 1 line · On Tue, Jul 22, 2014 at 11:14:26AM +0200, [email protected] wrote:

Hi Sascha,

(Adding back the list to Cc)

Quoted 22 lines
Quoted 6 lines
Quoted 6 lines
Signed-off-by Sebastian Block <[email protected]>

---
arch/arm/lib/bootm.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 15 lines

arch/arm/lib/bootm.c static int do_bootz_linux(struct image_data *data) · line 317

Quoted 10 lines
arch/arm/lib/bootm.c +1 -1
index 4896d01..d9b9111 100644
@@ -314,7 +314,7 @@ static int do_bootz_linux(struct image_data *data)
 			image_size - sizeof(*header));
 	if (ret < 0)
 		goto err_out;
-	if (ret < end - sizeof(*header)) {
+	if (ret < image_size - sizeof(*header)) {

Did this cause any real trouble? I was quite surprised when I saw that patch. After a test I saw that for me 'image_size' has the same value as
'end'.

Quoted 2 lines

I run into trouble with it, caused by variable end contains memory address of the image not the size as returned by read_full.

Hm, this must have something to do with your kernel configuration. Maybe you have CONFIG_ARM_PATCH_PHYS_VIRT disabled in your kernel.

Anyway, applied this patch to current master.

Sascha

Signature · 9 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox