On Fri, Oct 21, 2022 at 09:57:45PM +0530, Sughosh Ganu wrote: > hi Ilias, > > On Fri, 21 Oct 2022 at 20:33, Ilias Apalodimas > <ilias.apalodi...@linaro.org> wrote: > > > > Hi Sughosh > > > > > +{ > > > + int ret; > > > + u32 len, blk_start, blkcnt; > > > + struct disk_partition info; > > > + > > > + ALLOC_CACHE_ALIGN_BUFFER_PAD(struct fwu_mdata, mdata_aligned, 1, > > > + desc->blksz); > > > + > > > + if (!mdata) > > > + return -ENOMEM; > > > > ENOMEM is usually for allocation failures this is an -EINVAL > > > > > + > > > + ret = gpt_get_mdata_disk_part(desc, &info, part_num); > > > + if (ret < 0) { > > > + printf("Unable to get the FWU metadata partition\n"); > > > + return -ENOENT; > > > + } > > > + > > > + len = sizeof(*mdata); > > > + blkcnt = BLOCK_CNT(len, desc); > > > + if (blkcnt > info.size) { > > > + log_debug("Block count exceeds FWU metadata partition > > > size\n"); > > > + return -ERANGE; > > > + } > > > + > > > + blk_start = info.start; > > > + if (access == MDATA_READ) { > > > + if (blk_dread(desc, blk_start, blkcnt, mdata_aligned) != > > > blkcnt) { > > > + log_debug("Error reading FWU metadata from the > > > device\n"); > > > + return -EIO; > > > + } > > > + memcpy(mdata, mdata_aligned, sizeof(struct fwu_mdata)); > > > + } else { > > > > else if ? > > > > > + if (blk_dwrite(desc, blk_start, blkcnt, mdata) != blkcnt) { > > > + log_debug("Error writing FWU metadata to the > > > device\n"); > > > + return -EIO; > > > + } > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +static int fwu_gpt_update_mdata(struct udevice *dev, struct fwu_mdata > > > *mdata) > > > +{ > > > + int ret; > > > + struct blk_desc *desc; > > > + uint mdata_parts[2]; > > > + struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev); > > > + > > > + desc = dev_get_uclass_plat(priv->blk_dev); > > > > dev_get_uclass_plat might return NULL, gpt_read_write_mdata() doesn't check > > against NULL and then it ends up calling gpt_get_mdata_disk_part() which > > then calls part_get_info(). I don't think anyone checks for the desc ptr > > > > And I think this is a problem overall in all the callbacks belowe that > > invoke dev_get_uclass_plat() > > dev_get_uclass_plat() returns NULL only when the dev parameter passed > to the function is NULL. And that won't happen since that would mean > that the mdata_dev is NULL, and in such a scenario the driver's probe > function would fail. So these functions would not get called with desc > being NULL. > > -sughosh
Ok makes sense. Acked-by: Ilias Apalodimas <ilias.apalodi...@linaro.org> Thanks /Ilias > > > [...]