[LinuxBIOS] patch: extending LAR, and removing elf from linuxbios (it is not needed)

[Carl-Daniel Hailfinger]

On 29.08.2007 17:32, ron minnich wrote:
> R481 for LAR
> R482 for arch/x86 mods
> 
> Attached is a patch for 'noelf mode' + a FIX that is clean for lib/lar.c.
> 
> signed-off in patch.
> 
> ron
> 
> 
> ------------------------------------------------------------------------
> 
> This patch is for two things that are too hard to seperate, as they affect
> one common file.
> 
> The first is the noelf option, which is a simple
> modification to look for payloads in the form
> payload/segmentX where X is a number.
> 
> The second is the fix for improperly wrapping to 0 when
> searching the LAR.
> 
> This has been tested with filo and BOCHS.
> 
> Signed-off-by: Ronald G. Minnich <rminnich at gmail.com>

I like it. One minor comment:

> --- lib/lar.c	(revision 480)
> +++ lib/lar.c	(working copy)
> @@ -31,6 +31,13 @@
>  #define ntohl(x) (x)
>  #endif
>  
> +int run_address(void *f)
> +{
> +	int (*v) (void);
> +	v = f;
> +	return v();
> +}
> +
>  int find_file(struct mem_file *archive, char *filename, struct mem_file *result)
>  {
>  	char *walk, *fullname;
> @@ -40,31 +47,104 @@
>  	printk(BIOS_SPEW, "LAR: Start %p len 0x%x\n", archive->start,
>  	       archive->len);
>  
> +	/* Getting this for loop right is harder than it looks. All quantities are 
> +	  * unsigned. The LAR stretches from (e.g.) 0xfff0000 for 0x100000 
> +	  * bytes, i.e. to address ZERO. 
> +	  * As a result, 'walk', can wrap to zero and keep going (this has been 
> +	  * seen in practice). Recall that these are unsigned; walk can 
> +	  * wrap to zero; so, question, when is walk less than any of these:
> +	  * archive->start
> +	  * Answer: once walk wraps to zero, it is < archive->start
> +	  * archive->start + archive->len
> +	  * archive->start + archive->len  - 1
> +	  * Answer: In the case that archive->start + archive->len == 0, ALWAYS!
> +	  * A lot of expressions have been tried and all have been wrong. 
> +	  * So what would work? Simple:
> +	  * test for walk < archive->start + archive->len - 1 to cover the case
> +	  *	that the archive does NOT occupy ALL of the top of memory and 
> +	  *	wrap to zero; 
> +	  * and test for walk >= archive->start, 
> +	  * to cover the case that you wrapped to zero. 
> +	  * Unsigned pointer arithmetic that wraps to zero can be messy.
> +	  */
>  	for (walk = archive->start;
> -	     (walk - 1) < (char *)(archive->start + archive->len - 1 ); walk += 16) {
> +	     (walk < (char *)(archive->start + archive->len - 1)) && 
> +			(walk >= (char *)archive->start); walk += 16) {

What about

>  	for (walk = archive->start;
> -	     (walk - 1) < (char *)(archive->start + archive->len - 1 ); walk += 16) {
> +	     (walk <= (char *)(archive->start + archive->len - sizeof(struct lar_header))) && 
> +			(walk >= (char *)archive->start); walk += 16) {

instead? Looking for a lar header 1 byte before the end of the archive
is not going to succeed anyway.
Should we additionally check at the beginning of the function that the
archive is at least as long as sizeof(struct lar_header)?

>  		if (strcmp(walk, MAGIC) != 0)
>  			continue;
>  
>  		header = (struct lar_header *)walk;
>  		fullname = walk + sizeof(struct lar_header);
>  
> -		printk(BIOS_SPEW, "LAR: current filename is %s\n", fullname);
> +		printk(BIOS_SPEW, "LAR: search for %s\n", fullname);
>  		// FIXME: check checksum
>  
>  		if (strcmp(fullname, filename) == 0) {
> +			printk(BIOS_SPEW, "LAR: CHECK %s @ %p\n", fullname, header);
>  			result->start = walk + ntohl(header->offset);
>  			result->len = ntohl(header->len);
>  			result->reallen = ntohl(header->reallen);
>  			result->compression = ntohl(header->compression);
> +			result->entry = (void *)ntohl(header->entry);
> +			result->loadaddress = (void *)ntohl(header->loadaddress);
> +			printk(BIOS_SPEW, 
> +			"start %p len %d reallen %d compression %x entry %p loadaddress %p\n", 
> +				result->start, result->len, result->reallen, 
> +				result->compression, result->entry, result->loadaddress);
>  			return 0;
>  		}
>  		// skip file
>  		walk += (ntohl(header->len) + ntohl(header->offset) -
>  			1) & 0xfffffff0;
>  	}
> +	printk(BIOS_SPEW, "NO FILE FOUND\n");
>  	return 1;
>  }
>  

Otherwise, this patch and included comments look really nice.

Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>