On Friday 31 of August 2012 14:57:41 Marek Vasut wrote:
> > +lbaint_t sata_read(int dev, lbaint_t start, lbaint_t blkcnt, > > void *buffer) > > +{ > > + block_dev_desc_t *pdev = &(sata_dev_desc[dev]); > > + int fd = (long) pdev->priv; > > If pdev is NULL, this will crash
well, it isn't, at least not from the command - thats why you define the number of ports in advance, you get "dev" already range-checked
Range check is fine, but will pdev be inited? It's a pointer from some array.
init_sata is called first, so pdev is inited (see cmd_sata.c)
Unless it fails. Then what ?
the only way init can "fail" is if it gets a wrong device number (which should not happen), or if it cannot open the file, in which case it still sets pdev as -1.
If pdev is -1, then this explodes for certain, on unaligned access and on wrong ptr deref.
again, no
there is no pointer in this, pdev->priv is actually an int stored as void*, pdev itself is pointer to a global array. which one does explode in your case?
> > + memcpy(pdev->product, filenames[dev], namelen); > > + pdev->product[20] = 0; > > + > > + if (fd != -1) { > > And if "fd" is -1 ?
then all defaults to an invalid device, because you failed to open the file, for whatever the reason.
At least the printf below will choke, since pdev->lba is uninited
not the case. sata_dev_desc is inited in cmd_sata.c, and therefore by not doing anything we get an empty device
I see ... shall we also move all these memcpy() calls in to if (fd != -1) then?
I'd like to know that the device is a loopback, and what filename, not just that it failed to init
But are such data used somewhere further down the road?
yes, "sata info" for example
Pavel Herrmann