Hi, Leon KUKOVEC wrote:
Hi Wolfgang,
On Mon, 2004-03-15 at 14:32, Wolfgang Denk wrote:
In message 4055AA27.3010805@staubli.com you wrote:
The attached patch fixes a bug in the ide identification string copy.
I may be missing something - but why do we copy these strings in the first place? Who is using them? (and where?)
The block_dev_desc_t structure defines vendor[40], product[20] and revision[8] - same as it is defined in hd_driveid_t.
ident_cpy's description says that it will copy the string and terminate it, but it can't in the case where all 8 bytes of revision are occupied - since the buffers are equal size.
My hard disk has a 8 bytes revision string. In this case, u-boot crashes in init_part because the block_read field of the block_dev_desc_t structure has been partially erased by the ending null of the revision string. This crash didn't occured with the revison 1.5 of include/part.h because the field removable was beetween fields revision and block_read.
Therefore Pierre's patch is correct, but instead of hardcoded values 8, 40, 20 I would rather see sizeof (iop->fw_rev) etc.
The attached patch replaces my previous with this modification.
I noticed the same thing today when I was testing the PCMCIA patch that I'm about to send for PXA.
Best regards
diff -aur u-boot/common/cmd_ide.c u-boot-fix/common/cmd_ide.c --- u-boot/common/cmd_ide.c 2004-03-14 23:25:44.000000000 +0100 +++ u-boot-fix/common/cmd_ide.c 2004-03-16 08:55:24.000000000 +0100 @@ -1098,9 +1098,9 @@
input_swap_data (device, iobuf, ATA_SECTORWORDS);
- ident_cpy (dev_desc->revision, iop->fw_rev, sizeof(dev_desc->revision)); - ident_cpy (dev_desc->vendor, iop->model, sizeof(dev_desc->vendor)); - ident_cpy (dev_desc->product, iop->serial_no, sizeof(dev_desc->product)); + ident_cpy (dev_desc->revision, iop->fw_rev, sizeof (iop->fw_rev)); + ident_cpy (dev_desc->vendor, iop->model, sizeof (iop->model)); + ident_cpy (dev_desc->product, iop->serial_no, sizeof (iop->serial_no)); #ifdef __LITTLE_ENDIAN /* * firmware revision and model number have Big Endian Byte diff -aur u-boot/include/part.h u-boot-fix/include/part.h --- u-boot/include/part.h 2004-03-14 23:25:50.000000000 +0100 +++ u-boot-fix/include/part.h 2004-03-15 14:03:58.000000000 +0100 @@ -37,9 +37,9 @@ #endif unsigned long lba; /* number of blocks */ unsigned long blksz; /* block size */ - unsigned char vendor[40]; /* IDE model, SCSI Vendor */ - unsigned char product[20]; /* IDE Serial no, SCSI product */ - unsigned char revision[8]; /* firmware revision */ + unsigned char vendor[41]; /* IDE model, SCSI Vendor */ + unsigned char product[21]; /* IDE Serial no, SCSI product */ + unsigned char revision[9]; /* firmware revision */ unsigned long (*block_read)(int dev, unsigned long start, unsigned long blkcnt,