| Message ID | 1432202165-22351-5-git-send-email-shengyong1@huawei.com |
|---|---|
| State | RFC |
| Headers | show |
Am 21.05.2015 um 11:56 schrieb Sheng Yong: > When writing fastmap, there is no guarantee that two same vol ID are > written to flash by mistake. Although this unlikely happens, we still > should check it when attaching. Not sure if I understand the error scenario. How can this happen? Did you hit that care or is this theoretical? Thanks, //richard
Am 21.05.2015 um 12:16 schrieb Richard Weinberger: > Am 21.05.2015 um 11:56 schrieb Sheng Yong: >> When writing fastmap, there is no guarantee that two same vol ID are >> written to flash by mistake. Although this unlikely happens, we still >> should check it when attaching. > > Not sure if I understand the error scenario. > How can this happen? ...thought a bit more about that. I think your fix is correct but unless I'm very mistaken one can hit the issue only by modifying the fastmap by hand. Thanks, //richard
Hi, Richard On 5/22/2015 4:19 AM, Richard Weinberger wrote: > Am 21.05.2015 um 12:16 schrieb Richard Weinberger: >> Am 21.05.2015 um 11:56 schrieb Sheng Yong: >>> When writing fastmap, there is no guarantee that two same vol ID are >>> written to flash by mistake. Although this unlikely happens, we still >>> should check it when attaching. >> >> Not sure if I understand the error scenario. >> How can this happen? I didn't hit the case, this was just found by codewalking. After I read more code and though about more unclean reboot and ECC scenario, I agree with you that there is no way to have two same vol_id saved in different slots in ubi->volumes[], unless modify it by hand :( . Thank you for pointing it out. thanks, Sheng > > ...thought a bit more about that. > I think your fix is correct but unless I'm very > mistaken one can hit the issue only by modifying the fastmap > by hand. > > Thanks, > //richard > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ > >
Am 26.05.2015 um 10:53 schrieb Sheng Yong: > Hi, Richard > > On 5/22/2015 4:19 AM, Richard Weinberger wrote: >> Am 21.05.2015 um 12:16 schrieb Richard Weinberger: >>> Am 21.05.2015 um 11:56 schrieb Sheng Yong: >>>> When writing fastmap, there is no guarantee that two same vol ID are >>>> written to flash by mistake. Although this unlikely happens, we still >>>> should check it when attaching. >>> >>> Not sure if I understand the error scenario. >>> How can this happen? > I didn't hit the case, this was just found by codewalking. After I read > more code and though about more unclean reboot and ECC scenario, I agree > with you that there is no way to have two same vol_id saved in different > slots in ubi->volumes[], unless modify it by hand :( . Thank you for > pointing it out. Okay. :) Please resend your patch with a proper changelog where you explain that this issue can only be triggered by manually editing the on-flash data. Thanks, //richard
diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c index e3d8294..9eee904 100644 --- a/drivers/mtd/ubi/fastmap.c +++ b/drivers/mtd/ubi/fastmap.c @@ -192,8 +192,10 @@ static struct ubi_ainf_volume *add_vol(struct ubi_attach_info *ai, int vol_id, if (vol_id > av->vol_id) p = &(*p)->rb_left; - else + else if (vol_id < av->vol_id) p = &(*p)->rb_right; + else + return ERR_PTR(-EINVAL); } av = kmalloc(sizeof(struct ubi_ainf_volume), GFP_KERNEL); @@ -748,6 +750,11 @@ static int ubi_attach_fastmap(struct ubi_device *ubi, if (!av) goto fail_bad; + if (PTR_ERR(av) == -EINVAL) { + ubi_err(ubi, "volume (ID %i) already exists", + fmvhdr->vol_id); + goto fail_bad; + } ai->vols_found++; if (ai->highest_vol_id < be32_to_cpu(fmvhdr->vol_id))
When writing fastmap, there is no guarantee that two same vol ID are written to flash by mistake. Although this unlikely happens, we still should check it when attaching. Signed-off-by: Sheng Yong <shengyong1@huawei.com> --- drivers/mtd/ubi/fastmap.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)