[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Timur Tabi]

The do_bootm_linux() function uses the same variable ('len') to calculate the
dtu (flat device tree wrapped in a uImage) length and the initrd length, which
means that the initrd length was incorrect when it was needed.  This patch
changes any code that uses 'len' or 'data' as temporary variables to use
the values directly instead.  It also adds a comment to the definitions
of 'len' and 'data' reminding other programmers what they represent.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

This patch is a simplied version of the initrd patch that only fixes the root
problem, without renaming any variables.  This bug was introduced in commit
87a449c8ac396420cb24260f717ea9e6faa82047.

 common/cmd_bootm.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index 32c29e5..bd6d1c9 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -523,11 +523,11 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
 		int	verify)
 {
 	ulong	sp;
-	ulong	len, checksum;
+	ulong	len, data;	/* The initrd length and address */
 	ulong	initrd_start, initrd_end;
 	ulong	cmd_start, cmd_end;
 	ulong	initrd_high;
-	ulong	data;
+	ulong	checksum;
 	int	initrd_copy_to_ram = 1;
 	char    *cmdline;
 	char	*s;
@@ -652,13 +652,10 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
 			do_reset (cmdtp, flag, argc, argv);
 		}

-		data = (ulong)&header;
-		len  = sizeof(image_header_t);
-
 		checksum = ntohl(hdr->ih_hcrc);
 		hdr->ih_hcrc = 0;

-		if (crc32 (0, (uchar *)data, len) != checksum) {
+		if (crc32 (0, (uchar *) &header, sizeof(image_header_t)) != checksum) {
 			puts ("Bad Header Checksum\n");
 			SHOW_BOOT_PROGRESS (-11);
 			do_reset (cmdtp, flag, argc, argv);
@@ -779,9 +776,8 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,

 			checksum = ntohl(hdr->ih_dcrc);
 			addr = (ulong)((uchar *)(hdr) + sizeof(image_header_t));
-			len = ntohl(hdr->ih_size);

-			if(checksum != crc32(0, (uchar *)addr, len)) {
+			if(checksum != crc32(0, (uchar *)addr, ntohl(hdr->ih_size))) {
 				printf("ERROR: Flat Device Tree checksum is invalid\n");
 				return;
 			}
--
1.5.0.2.260.g2eb065

[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Wolfgang Denk]

Dear Timur,

in message <11782314361072-git-send-email-timur@freescale.com> you wrote:
> The do_bootm_linux() function uses the same variable ('len') to calculate the
> dtu (flat device tree wrapped in a uImage) length and the initrd length, which
> means that the initrd length was incorrect when it was needed.  This patch
> changes any code that uses 'len' or 'data' as temporary variables to use
> the values directly instead.  It also adds a comment to the definitions
> of 'len' and 'data' reminding other programmers what they represent.
...
> This patch is a simplied version of the initrd patch that only fixes the root
> problem, without renaming any variables.  This bug was introduced in commit
> 87a449c8ac396420cb24260f717ea9e6faa82047.
...
> @@ -523,11 +523,11 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
>  		int	verify)
>  {
>  	ulong	sp;
> -	ulong	len, checksum;
> +	ulong	len, data;	/* The initrd length and address */
>  	ulong	initrd_start, initrd_end;
>  	ulong	cmd_start, cmd_end;
>  	ulong	initrd_high;
> -	ulong	data;
> +	ulong	checksum;
>  	int	initrd_copy_to_ram = 1;
>  	char    *cmdline;
>  	char	*s;

So before your patch we had "len", "checksum", and "data", and now we
have "len", "data", and "checksum".

What is your reason for this change? I see  no  functinal  difference
nor any improvement of readability.


> @@ -652,13 +652,10 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
>  			do_reset (cmdtp, flag, argc, argv);
>  		}
> 
> -		data = (ulong)&header;
> -		len  = sizeof(image_header_t);
> -
>  		checksum = ntohl(hdr->ih_hcrc);
>  		hdr->ih_hcrc = 0;
> 
> -		if (crc32 (0, (uchar *)data, len) != checksum) {
> +		if (crc32 (0, (uchar *) &header, sizeof(image_header_t)) != checksum) {

This looks functionally identical to me, but is less readable.

> @@ -779,9 +776,8 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
> 
>  			checksum = ntohl(hdr->ih_dcrc);
>  			addr = (ulong)((uchar *)(hdr) + sizeof(image_header_t));
> -			len = ntohl(hdr->ih_size);
> 
> -			if(checksum != crc32(0, (uchar *)addr, len)) {
> +			if(checksum != crc32(0, (uchar *)addr, ntohl(hdr->ih_size))) {

Same here. functionally identical but less readable.


With this patch it is impossible to see what the problem is  you  are
trying to fix.

Looking at the patch, I see only cosmetical changes, which actually
lead to less readable code.


If there is a problem,  please  fix  the  problem,  but  don't  touch
innocent code that has been there for ages.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,    CEO: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I think there's a world market for about five computers.
         -- attr. Thomas J. Watson (Chairman of the Board, IBM), 1943

[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Timur Tabi]

Wolfgang Denk wrote:

> So before your patch we had "len", "checksum", and "data", and now we
> have "len", "data", and "checksum".
> 
> What is your reason for this change? I see  no  functinal  difference
> nor any improvement of readability.

It's for the comment that I added.  I wanted to make sure that the reader understood what 
'len' and 'data' are for, since their purpose is not obvious from just reading the code.

>> -		if (crc32 (0, (uchar *)data, len) != checksum) {
>> +		if (crc32 (0, (uchar *) &header, sizeof(image_header_t)) != checksum) {
> 
> This looks functionally identical to me, but is less readable.

The point behind the patch is to make sure that 'len' and 'data' are only used for the 
purpose they are intended.  Here, len and data are used as temporary variables.


> 
>> @@ -779,9 +776,8 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
>>
>>  			checksum = ntohl(hdr->ih_dcrc);
>>  			addr = (ulong)((uchar *)(hdr) + sizeof(image_header_t));
>> -			len = ntohl(hdr->ih_size);
>>
>> -			if(checksum != crc32(0, (uchar *)addr, len)) {
>> +			if(checksum != crc32(0, (uchar *)addr, ntohl(hdr->ih_size))) {
> 
> Same here. functionally identical but less readable.

Not true.  In this case, len has already been assigned the length of the initrd, and here 
we lose this value.  Without this change, the kernel will panic at boot.

> With this patch it is impossible to see what the problem is  you  are
> trying to fix.

It's clearly explained in the comment: 'data' and 'len' are the address and length of the 
initrd, so this patch makes sure that these variables only contain those values.


-- 
Timur Tabi
Linux Kernel Developer @ Freescale

[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Wolfgang Denk]

Dear Timur,

in message <463A88BA.2030607@freescale.com> you wrote:
> 
> > So before your patch we had "len", "checksum", and "data", and now we
> > have "len", "data", and "checksum".
> > 
> > What is your reason for this change? I see  no  functinal  difference
> > nor any improvement of readability.
> 
> It's for the comment that I added.  I wanted to make sure that the reader understood what 
> 'len' and 'data' are for, since their purpose is not obvious from just reading the code.

Your comment is actually wrong. The use of "len" is not limited to
that purpose.

But that doesn't mean that you can overwrite  this  variable  without
checking.

> >> -		if (crc32 (0, (uchar *)data, len) != checksum) {
> >> +		if (crc32 (0, (uchar *) &header, sizeof(image_header_t)) != checksum) {
> > 
> > This looks functionally identical to me, but is less readable.
> 
> The point behind the patch is to make sure that 'len' and 'data' are only used for the 
> purpose they are intended.  Here, len and data are used as temporary variables.

...and here this is perfectly legal, as  "len"  does  not  store  any
other value that is needed later.

> >> @@ -779,9 +776,8 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
> >>
> >>  			checksum = ntohl(hdr->ih_dcrc);
> >>  			addr = (ulong)((uchar *)(hdr) + sizeof(image_header_t));
> >> -			len = ntohl(hdr->ih_size);
> >>
> >> -			if(checksum != crc32(0, (uchar *)addr, len)) {
> >> +			if(checksum != crc32(0, (uchar *)addr, ntohl(hdr->ih_size))) {
> > 
> > Same here. functionally identical but less readable.
> 
> Not true.  In this case, len has already been assigned the length of the initrd, and here 
> we lose this value.  Without this change, the kernel will panic at boot.

Right. I agree that this is the core of the problem,  but  from  your
patch  it was impossible to see. You have to read the diff, plus your
patch description, plus the code.


I think we should just fix the bug, which is the incorrect use of the
variable "len" at a place where it was already used. As Johns  Daniel
pointed  out  (see his message from Tue, 27 Mar 2007 11:25:55 -0500),
this is the core of your poatch and all that needs to be changed.

> > With this patch it is impossible to see what the problem is  you  are
> > trying to fix.
> 
> It's clearly explained in the comment: 'data' and 'len' are the address and length of the 
> initrd, so this patch makes sure that these variables only contain those values.

No, this explanation is wrong. "len" gets used for some other
purposes, too.


To put this problem finally to rest I checked in the fix as suggested
by Johns Daniel, i. e. the core idea of your patch. See
http://www.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commit;h=9877d7dcd1eebe61aa5d8b8ffe9c048ea426e6f6


Thanks for pointing out the problem and providing a fix.

And please accept my apologies thatt his was so complicated and  took
so  long.  [Nevertheless you still might want to try to find a way to
access the repository I  created  for  you  in  case  you  have  more
patches.]

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,    CEO: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"My name is Linus Torvalds, you messed with my kernel, prepare to die"
                                                  - Linus Torvalds in
      <Pine.LNX.3.91.960426110644.24860I-100000@linux.cs.Helsinki.FI>

[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Timur Tabi]

Wolfgang Denk wrote:

I'm glad you fixed the bug, so I'll just add a few comments:

> Your comment is actually wrong. The use of "len" is not limited to
> that purpose.

If you apply my patch, then the comment becomes correct.  My goal was to 
lock the variables 'len' and 'data' into one purpose.  The reason the 
bug existed is because the other developer didn't realize this.  He used 
'len' thinking it was available.  In a sense, I was trying to implement 
some "defensive programming", so that the next time someone hacks up 
do_bootm_linux(), he won't re-introduce the bug.

Now, you might say that that won't happen again, but I disagree.  I 
think it can, for two reasons:

1) It happened once already, last year.  You approved and applied a 
patch which does overwrite the variable.

2) The libfdt code introduced a number of other bugs relating to dtu 
usage, which have not yet been fixed.

So I believe there is a real possibility that another patch could 
re-introduce this bug.  If you had applied my patch as-is, that 
possibility would have been eliminated.  This is why I think my patch is 
better than yours.

But I guess only time will tell who's right. :-)

> And please accept my apologies thatt his was so complicated and  took
> so  long.  [Nevertheless you still might want to try to find a way to
> access the repository I  created  for  you  in  case  you  have  more
> patches.]

Stefan said he had a testing repo of some kind.  How about we just use 
that?  If Stefan is willing, he can apply my emailed patches to his repo.

[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Stefan Roese]

Hi Timur,

On Friday 04 May 2007 15:25, Timur Tabi wrote:
> I'm glad you fixed the bug, so I'll just add a few comments:
> > Your comment is actually wrong. The use of "len" is not limited to
> > that purpose.
>
> If you apply my patch, then the comment becomes correct.  My goal was to
> lock the variables 'len' and 'data' into one purpose.  The reason the
> bug existed is because the other developer didn't realize this.  He used
> 'len' thinking it was available.  In a sense, I was trying to implement
> some "defensive programming", so that the next time someone hacks up
> do_bootm_linux(), he won't re-introduce the bug.
>
> Now, you might say that that won't happen again, but I disagree.  I
> think it can, for two reasons:
>
> 1) It happened once already, last year.  You approved and applied a
> patch which does overwrite the variable.
>
> 2) The libfdt code introduced a number of other bugs relating to dtu
> usage, which have not yet been fixed.
>
> So I believe there is a real possibility that another patch could
> re-introduce this bug.  If you had applied my patch as-is, that
> possibility would have been eliminated.  This is why I think my patch is
> better than yours.
>
> But I guess only time will tell who's right. :-)

You have a point with your variable usage "restriction", and Wolfgang has a 
point with the "readability" of your patch as it doesn't really tell what is 
fixed without read the current source. Could be that your implementation is 
the "better" one, but the patch Wolfgang applied was just less intrusive. 

> > And please accept my apologies thatt his was so complicated and  took
> > so  long.  [Nevertheless you still might want to try to find a way to
> > access the repository I  created  for  you  in  case  you  have  more
> > patches.]
>
> Stefan said he had a testing repo of some kind.  How about we just use
> that?  If Stefan is willing, he can apply my emailed patches to his repo.

No, we shouldn't "fork" the code here. Let's focus on the other open issues. 
You mention other bugs introduced by the libfdt code above. ;-)

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
=====================================================================

[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Jerry Van Baren]

Stefan Roese wrote:
> Hi Timur,
> 
> On Friday 04 May 2007 15:25, Timur Tabi wrote:
>> I'm glad you fixed the bug, so I'll just add a few comments:
>>> Your comment is actually wrong. The use of "len" is not limited to
>>> that purpose.
>> If you apply my patch, then the comment becomes correct.  My goal was to
>> lock the variables 'len' and 'data' into one purpose.  The reason the
>> bug existed is because the other developer didn't realize this.  He used
>> 'len' thinking it was available.  In a sense, I was trying to implement
>> some "defensive programming", so that the next time someone hacks up
>> do_bootm_linux(), he won't re-introduce the bug.
>>
>> Now, you might say that that won't happen again, but I disagree.  I
>> think it can, for two reasons:
>>
>> 1) It happened once already, last year.  You approved and applied a
>> patch which does overwrite the variable.
>>
>> 2) The libfdt code introduced a number of other bugs relating to dtu
>> usage, which have not yet been fixed.
>>
>> So I believe there is a real possibility that another patch could
>> re-introduce this bug.  If you had applied my patch as-is, that
>> possibility would have been eliminated.  This is why I think my patch is
>> better than yours.
>>
>> But I guess only time will tell who's right. :-)
> 
> You have a point with your variable usage "restriction", and Wolfgang has a 
> point with the "readability" of your patch as it doesn't really tell what is 
> fixed without read the current source. Could be that your implementation is 
> the "better" one, but the patch Wolfgang applied was just less intrusive. 
> 
>>> And please accept my apologies thatt his was so complicated and  took
>>> so  long.  [Nevertheless you still might want to try to find a way to
>>> access the repository I  created  for  you  in  case  you  have  more
>>> patches.]
>> Stefan said he had a testing repo of some kind.  How about we just use
>> that?  If Stefan is willing, he can apply my emailed patches to his repo.
> 
> No, we shouldn't "fork" the code here. Let's focus on the other open issues. 
> You mention other bugs introduced by the libfdt code above. ;-)
> 
> Best regards,
> Stefan

Timur has sent me two libfdt/dtu-related fixes
* fdt_copy_into() had the source and destination addresses reversed
* fdt_check_header() had the wrong sense on ==/!= 0
which I applied to my local repo last night and will push back to 
u-boot-fdt soon (I still have not been successful in figuring out how to 
make a multi-image to test the changes grrrr).  The two bugs I 
introduced in the conversion to libfdt were unrelated to the "len" 
variable, but I probably replicated the "len" bug when I duplicated and 
modified the original code.  I will pull Wolfgang's fix tonight and see 
what needs to be done to make all three bugs go away.

WRT to building a multi-image, I'm looking to combine linux, a dtb, and 
an initrd into a single image to test that path of bootm (the path I had 
the above screwups in).

Timur's hint for me was:
> mkimage -A ppc -T flat_dt -C none -d mpc836x_mds.dtb mpc836x_mds.dtu
> 
> This, of course, creates a dtu with a value of 0 for ih_load.  You
> probably need to specify -a or -e to set that field.
but I don't understand how to build an image with all three pieces in it 
(I tried to use the ":" in the -d source parameter and mkimage just got 
confused, couldn't find the files).  Am I expecting too much???  Should 
I just be wrapping the three pieces individually and loading them 
separately?  What exactly are you doing to test this, Timur?

Best regards,
gvb

[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Timur Tabi]

Stefan Roese wrote:

> You have a point with your variable usage "restriction", and Wolfgang has a 
> point with the "readability" of your patch as it doesn't really tell what is 
> fixed without read the current source. Could be that your implementation is 
> the "better" one, but the patch Wolfgang applied was just less intrusive. 

Well, I can't argue with that.

> No, we shouldn't "fork" the code here. Let's focus on the other open issues. 

Since you bring it up, I submitted a patch for the 5xxx platform many months ago.  How do 
I get that applied?  The patch is here:

http://sourceforge.net/mailarchive/message.php?msg_id=117131648887-git-send-email-timur%40freescale.com

> You mention other bugs introduced by the libfdt code above. ;-)

Jerry is already aware of them and working on fixes.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Timur Tabi]

Jerry Van Baren wrote:

> but I don't understand how to build an image with all three pieces in it 
> (I tried to use the ":" in the -d source parameter and mkimage just got 
> confused, couldn't find the files).  Am I expecting too much???  Should 
> I just be wrapping the three pieces individually and loading them 
> separately?  What exactly are you doing to test this, Timur?

The 'len' bug only shows up if both of these conditions are met:

1) You're booting an OF-enabled kernel (i.e. there's an fdt)
2) The fdt is wrapped in a dtu image (type IH_TYPE_FLATDT)

I didn't test having the fdt merged in with other entities in a combined image.  I just 
made a dtu and told the bootm command to use it.  So if you want to test this code, I 
think just wrapping the three pieces individually should be sufficient.

Part of the problem is that the code looks for a dtu image.  If you combine all three 
chunks into one image, then I don't know what the code will do, because the image type 
won't be IH_TYPE_FLATDT.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Jerry Van Baren]

Timur Tabi wrote:
> Jerry Van Baren wrote:
> 
>> but I don't understand how to build an image with all three pieces in 
>> it (I tried to use the ":" in the -d source parameter and mkimage just 
>> got confused, couldn't find the files).  Am I expecting too much???  
>> Should I just be wrapping the three pieces individually and loading 
>> them separately?  What exactly are you doing to test this, Timur?
> 
> The 'len' bug only shows up if both of these conditions are met:
> 
> 1) You're booting an OF-enabled kernel (i.e. there's an fdt)
> 2) The fdt is wrapped in a dtu image (type IH_TYPE_FLATDT)
> 
> I didn't test having the fdt merged in with other entities in a combined 
> image.  I just made a dtu and told the bootm command to use it.  So if 
> you want to test this code, I think just wrapping the three pieces 
> individually should be sufficient.
> 
> Part of the problem is that the code looks for a dtu image.  If you 
> combine all three chunks into one image, then I don't know what the code 
> will do, because the image type won't be IH_TYPE_FLATDT.

Ahh, thank you.  That clarifies what I was misunderstanding.

gvb

[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Wolfgang Denk]

In message <463B3463.1030702@freescale.com> you wrote:
> 
> If you apply my patch, then the comment becomes correct.  My goal was to 
> lock the variables 'len' and 'data' into one purpose.  The reason the 

I think you actually missed a few places where len was also used.

> bug existed is because the other developer didn't realize this.  He used 
> 'len' thinking it was available.  In a sense, I was trying to implement 

This type of thinking  was  plain  wrong.  You  must  never  use  any
variable without understanding what it's being used for.

> Stefan said he had a testing repo of some kind.  How about we just use 
> that?  If Stefan is willing, he can apply my emailed patches to his repo.

The u-boot-testing repo is actually *my* guardian repo where I put all
stuff in that does not fit anywhere else.

As I explained before, you need a  normal  developer's  repo,  not  a
guardian  one.  And I repeat: I offer you such a repo, it's available
right now, I just need the IP address from where you will connect.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,    CEO: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Cogito cogito ergo cogito sum - "I think that I  think,  therefore  I
think that I am."          - Ambrose Bierce, "The Devil's Dictionary"

[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Timur Tabi]

Wolfgang Denk wrote:
> In message <463B3463.1030702@freescale.com> you wrote:
>> If you apply my patch, then the comment becomes correct.  My goal was to 
>> lock the variables 'len' and 'data' into one purpose.  The reason the 
> 
> I think you actually missed a few places where len was also used.

Really?  I doubt it.  Every other place that len is used, it's the initrd length.  Can you 
show me where that's not the case?

>> bug existed is because the other developer didn't realize this.  He used 
>> 'len' thinking it was available.  In a sense, I was trying to implement 
> 
> This type of thinking  was  plain  wrong.  You  must  never  use  any
> variable without understanding what it's being used for.

You're absolutely right, which is why *I* don't make that mistake.  But some one else did, 
and someone else might again.  do_bootm_linux() is hideous code, and it just gets worse 
and worse.  Frankly, anyone trying to modify that code needs all the help he can get!

> As I explained before, you need a  normal  developer's  repo,  not  a
> guardian  one.  And I repeat: I offer you such a repo, it's available
> right now, I just need the IP address from where you will connect.

It would be the same IP address that Kim, Andy, and Jon use.  As soon as I get hold of one 
of them, I'll ask, but technically you should know the answer already.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Wolfgang Denk]

In message <463B6022.7000908@freescale.com> you wrote:
>
> Really?  I doubt it.  Every other place that len is used, it's the initrd length.  Can you 
> show me where that's not the case?

See for example line 656:                 len  = sizeof(image_header_t);

> It would be the same IP address that Kim, Andy, and Jon use.  As soon as I get hold of one 

And that is???

> of them, I'll ask, but technically you should know the answer already.

No, I don't. That's why I ask.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,    CEO: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There are two kinds of people, those who do the work  and  those  who
take  the  credit.  Try  to  be  in  the  first  group; there is less
competition there.                                    - Indira Gandhi

[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Timur Tabi]

Wolfgang Denk wrote:
> In message <463B6022.7000908@freescale.com> you wrote:
>> Really?  I doubt it.  Every other place that len is used, it's the initrd length.  Can you 
>> show me where that's not the case?
> 
> See for example line 656:                 len  = sizeof(image_header_t);

Wolfgang, look at my patch!!!!

@@ -652,13 +652,10 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
  			do_reset (cmdtp, flag, argc, argv);
  		}

-		data = (ulong)&header;
-		len  = sizeof(image_header_t);
-
  		checksum = ntohl(hdr->ih_hcrc);
  		hdr->ih_hcrc = 0;

-		if (crc32 (0, (uchar *)data, len) != checksum) {
+		if (crc32 (0, (uchar *) &header, sizeof(image_header_t)) != checksum) {
  			puts ("Bad Header Checksum\n");
  			SHOW_BOOT_PROGRESS (-11);
  			do_reset (cmdtp, flag, argc, argv);


My patch deletes that usage of 'len'!!!  *Your* patch doesn't!

>> It would be the same IP address that Kim, Andy, and Jon use.  As soon as I get hold of one 
> 
> And that is???

I just spoke to Andy, and he never gave you the IP address comes from, so I don't 
understand why you need from him but didn't need it from him.  Regardless, I'm going to 
try to figure out the IP address, and I'll send you another email.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Timur Tabi]

Wolfgang Denk wrote:

>> It would be the same IP address that Kim, Andy, and Jon use.  As soon as I get hold of one 
> 
> And that is???

I *think* the IP address is 192.88.165.35, aka gate-de.freescale.com.  That's what 
external sites say my IP address is when I connect to them.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale