Re: [Orinoco-devel] [PATCH 1/2] orinoco: validate firmware header

[Dave <kilroyd@googlemail.com>]

Andrey Borzenkov wrote:
> On 20 of February 2009 02:46:26 David Kilroy wrote:
>> +static const char *fw_err[] = {
>> +	"image too small",
>> +};
>> +
>>
>> +/* Check the range of various header entries */
>> +static int validate_fw(const struct orinoco_fw_header *hdr, size_t
>> len) +{
>> +	if (len < sizeof(*hdr))
>> +		return 1;
>> +
>> +	/* TODO: consider adding a checksum or CRC to the firmware format
>> */ +	return 0;
>> +}
> 
> 
> I am afraid this can easily go off sync. Any reason those messages 
> cannot be printed inline in validate_fw()?

That's how I started, but I didn't like having to pass down struct
net_device/struct orinoco_private just to print an error.

> Otherwise what about
> 
> #define FW_ERR_OK	0
> #define FW_ERR_TOO_SMALL 1
> ...
> static const char *fw_err[] = {
> 	[FW_ERR_TOO_SMALL] = "image too small",
> ?

Sure. That (or an enumeration) documents the error codes. A little
tedious though. What if I did something like:

static char * validate_fw(...)
{
   if (len < sizeof(*hdr))
      return "image too small";
   ...

   return NULL;
}

...
{
   char *fw_err = validate_firmware(hdr, fw_entry->size);

   if (fw_err != NULL) {
       printk(KERN_WARNING "%s: Invalid firmware (%s)\n",
	      dev->name, fw_err)
       err = -EINVAL;
       goto abort;
   }
}


Dave.