Re: [BUG]: mm/vmalloc: uninitialized variable access in pcpu_get_vm_areas

From: Uladzislau Rezki
Date: Mon Jun 17 2019 - 13:02:40 EST


>
> I managed to un-confuse gcc-8 by turning the if/else if/else into
> a switch statement. If you all think this is an acceptable solution,
> I'll submit that after some more testing to ensure it addresses
> all configurations:
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index a9213fc3802d..5b7e50de008b 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -915,7 +915,8 @@ adjust_va_to_fit_type(struct vmap_area *va,
> {
> struct vmap_area *lva;
>
> - if (type == FL_FIT_TYPE) {
> + switch (type) {
> + case FL_FIT_TYPE:
> /*
> * No need to split VA, it fully fits.
> *
> @@ -925,7 +926,8 @@ adjust_va_to_fit_type(struct vmap_area *va,
> */
> unlink_va(va, &free_vmap_area_root);
> kmem_cache_free(vmap_area_cachep, va);
> - } else if (type == LE_FIT_TYPE) {
> + break;
> + case LE_FIT_TYPE:
> /*
> * Split left edge of fit VA.
> *
> @@ -934,7 +936,8 @@ adjust_va_to_fit_type(struct vmap_area *va,
> * |-------|-------|
> */
> va->va_start += size;
> - } else if (type == RE_FIT_TYPE) {
> + break;
> + case RE_FIT_TYPE:
> /*
> * Split right edge of fit VA.
> *
> @@ -943,7 +946,8 @@ adjust_va_to_fit_type(struct vmap_area *va,
> * |-------|-------|
> */
> va->va_end = nva_start_addr;
> - } else if (type == NE_FIT_TYPE) {
> + break;
> + case NE_FIT_TYPE:
> /*
> * Split no edge of fit VA.
> *
> @@ -980,7 +984,8 @@ adjust_va_to_fit_type(struct vmap_area *va,
> * Shrink this VA to remaining size.
> */
> va->va_start = nva_start_addr + size;
> - } else {
> + break;
> + default:
> return -1;
> }
>
To me it is not clear how it would solve the warning. It sounds like
your GCC after this change is able to keep track of that variable
probably because of less generated code. But i am not sure about
other versions. For example i have:

gcc version 6.3.0 20170516 (Debian 6.3.0-18+deb9u1)

and it totally OK, i.e. it does not emit any related warning.

Another thing is that, if we add mode code there or change the function
prototype, we might run into the same warning. Therefore i proposed that
we just set the variable to NULL, i.e. Initialize it.

<snip>
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index b1bb5fc6eb05..10cfb93aba1e 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -913,7 +913,11 @@ adjust_va_to_fit_type(struct vmap_area *va,
unsigned long nva_start_addr, unsigned long size,
enum fit_type type)
{
- struct vmap_area *lva;
+ /*
+ * Some GCC versions can emit bogus warning that it
+ * may be used uninitialized, therefore set it NULL.
+ */
+ struct vmap_area *lva = NULL;

if (type == FL_FIT_TYPE) {
/*
<snip>

--
Vlad Rezki