Dear Pavel Herrmann,
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?
pdev, but given that the pointer is inited, I see the issue at hand now.
> > > + 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
And how does it determine that the init failed?
I think we're almost clear on these things.
Pavel Herrmann
Best regards, Marek Vasut