[PATCH] UBI: Fix crash in try_recover_peb()

[Geert Uytterhoeven]

drivers/mtd/ubi/eba.c: In function ‘try_recover_peb’:
drivers/mtd/ubi/eba.c:744: warning: ‘vid_hdr’ is used uninitialized in this function

The pointer vid_hdr is indeed not initialized, leading to a crash when
it is dereferenced.

Fix this by obtaining the pointer from the VID buffer, like is done
everywhere else.

Fixes: 3291b52f9ff0acc8 ("UBI: introduce the VID buffer concept")
Signed-off-by: Geert Uytterhoeven <***@linux-m68k.org>
---
Completely untested. And I know nothing about UBI ;-)
---
drivers/mtd/ubi/eba.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
index 95c4048a371e87b6..388e46be6ad92805 100644
--- a/drivers/mtd/ubi/eba.c
+++ b/drivers/mtd/ubi/eba.c
@@ -741,6 +741,7 @@ static int try_recover_peb(struct ubi_volume *vol, int pnum, int lnum,
goto out_put;
}

+ vid_hdr = ubi_get_vid_hdr(vidb);
ubi_assert(vid_hdr->vol_type == UBI_VID_DYNAMIC);

mutex_lock(&ubi->buf_mutex);

[Boris Brezillon]

Hi Geert,

On Thu, 13 Oct 2016 16:05:36 +0200

Post by Geert Uytterhoeven
drivers/mtd/ubi/eba.c:744: warning: ‘vid_hdr’ is used uninitialized in this function
The pointer vid_hdr is indeed not initialized, leading to a crash when
it is dereferenced.
Fix this by obtaining the pointer from the VID buffer, like is done
everywhere else.

Indeed, I don't know how I missed that one :-/.

Post by Geert Uytterhoeven
Fixes: 3291b52f9ff0acc8 ("UBI: introduce the VID buffer concept")

One minor comment below, otherwise

Post by Geert Uytterhoeven
---
Completely untested. And I know nothing about UBI ;-)
---
drivers/mtd/ubi/eba.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
index 95c4048a371e87b6..388e46be6ad92805 100644
--- a/drivers/mtd/ubi/eba.c
+++ b/drivers/mtd/ubi/eba.c
@@ -741,6 +741,7 @@ static int try_recover_peb(struct ubi_volume *vol, int pnum, int lnum,
goto out_put;
}
+ vid_hdr = ubi_get_vid_hdr(vidb);

Can you move this assignment at variable declaration time?

Post by Geert Uytterhoeven
ubi_assert(vid_hdr->vol_type == UBI_VID_DYNAMIC);
mutex_lock(&ubi->buf_mutex);

[Boris Brezillon]

On Thu, 13 Oct 2016 16:24:58 +0200

Post by Boris Brezillon
Hi Geert,
On Thu, 13 Oct 2016 16:05:36 +0200

Post by Geert Uytterhoeven
drivers/mtd/ubi/eba.c:744: warning: ‘vid_hdr’ is used uninitialized in this function
The pointer vid_hdr is indeed not initialized, leading to a crash when
it is dereferenced.
Fix this by obtaining the pointer from the VID buffer, like is done
everywhere else.

Indeed, I don't know how I missed that one :-/.

That's really weird. I tried to recompile the kernel on my machine
(before applying your patch), to see if the compiler was complaining,
and it's not :-(.

I also coded simple functions with goto statements, to check that the
detection was working correctly, and it is.

Maybe you have some ideas why my compiler is failing to detect the
problem (I'm currently using the ubuntu cross-toolchain:
'arm-linux-gnueabi-gcc (Ubuntu/Linaro 5.4.0-6ubuntu1~16.04.1) 5.4.0
20160609').

Post by Boris Brezillon

Post by Geert Uytterhoeven
Fixes: 3291b52f9ff0acc8 ("UBI: introduce the VID buffer concept")

One minor comment below, otherwise

Post by Geert Uytterhoeven
---
Completely untested. And I know nothing about UBI ;-)
---
drivers/mtd/ubi/eba.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
index 95c4048a371e87b6..388e46be6ad92805 100644
--- a/drivers/mtd/ubi/eba.c
+++ b/drivers/mtd/ubi/eba.c
@@ -741,6 +741,7 @@ static int try_recover_peb(struct ubi_volume *vol, int pnum, int lnum,
goto out_put;
}
+ vid_hdr = ubi_get_vid_hdr(vidb);

Can you move this assignment at variable declaration time?

Post by Geert Uytterhoeven
ubi_assert(vid_hdr->vol_type == UBI_VID_DYNAMIC);
mutex_lock(&ubi->buf_mutex);