* [PATCH 2/3] mm: vmalloc: fix vmap_ram_vread_iter() return value
2023-03-23 11:58 [PATCH 1/3] mm/mmap/vma_merge: actually set next to NULL if not applicable Arnd Bergmann
@ 2023-03-23 11:58 ` Arnd Bergmann
2023-03-23 12:05 ` Lorenzo Stoakes
2023-03-23 11:58 ` [PATCH 3/3] mm: vmalloc: mark zero_iter() static Arnd Bergmann
2023-03-23 12:03 ` [PATCH 1/3] mm/mmap/vma_merge: actually set next to NULL if not applicable Lorenzo Stoakes
2 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2023-03-23 11:58 UTC (permalink / raw)
To: Andrew Morton, Lorenzo Stoakes
Cc: Arnd Bergmann, Uladzislau Rezki, Christoph Hellwig, Baoquan He,
David Hildenbrand, Andrey Konovalov, linux-mm, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
The error handling in vmap_ram_vread_iter() can return an
uninitialized value in some cases:
mm/vmalloc.c:3539:6: error: variable 'remains' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
if (!vb)
^~~
mm/vmalloc.c:3587:17: note: uninitialized use occurs here
return count - remains + zero_iter(iter, remains);
^~~~~~~
Move the initialization up a few lines.
Fixes: d9cab54f7737 ("mm: vmalloc: convert vread() to vread_iter()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
mm/vmalloc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index ebfa1e9fe6f9..fb216495fe5a 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -3521,7 +3521,7 @@ static size_t vmap_ram_vread_iter(struct iov_iter *iter, const char *addr,
struct vmap_block *vb;
unsigned long offset;
unsigned int rs, re;
- size_t remains, n;
+ size_t remains = count, n;
/*
* If it's area created by vm_map_ram() interface directly, but
@@ -3545,7 +3545,6 @@ static size_t vmap_ram_vread_iter(struct iov_iter *iter, const char *addr,
goto finished_zero;
}
- remains = count;
for_each_set_bitrange(rs, re, vb->used_map, VMAP_BBMAP_BITS) {
size_t copied;
--
2.39.2
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] mm: vmalloc: fix vmap_ram_vread_iter() return value
2023-03-23 11:58 ` [PATCH 2/3] mm: vmalloc: fix vmap_ram_vread_iter() return value Arnd Bergmann
@ 2023-03-23 12:05 ` Lorenzo Stoakes
0 siblings, 0 replies; 6+ messages in thread
From: Lorenzo Stoakes @ 2023-03-23 12:05 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Andrew Morton, Arnd Bergmann, Uladzislau Rezki,
Christoph Hellwig, Baoquan He, David Hildenbrand,
Andrey Konovalov, linux-mm, linux-kernel
On Thu, Mar 23, 2023 at 12:58:35PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The error handling in vmap_ram_vread_iter() can return an
> uninitialized value in some cases:
>
> mm/vmalloc.c:3539:6: error: variable 'remains' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
> if (!vb)
> ^~~
> mm/vmalloc.c:3587:17: note: uninitialized use occurs here
> return count - remains + zero_iter(iter, remains);
> ^~~~~~~
>
> Move the initialization up a few lines.
>
> Fixes: d9cab54f7737 ("mm: vmalloc: convert vread() to vread_iter()")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> mm/vmalloc.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index ebfa1e9fe6f9..fb216495fe5a 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -3521,7 +3521,7 @@ static size_t vmap_ram_vread_iter(struct iov_iter *iter, const char *addr,
> struct vmap_block *vb;
> unsigned long offset;
> unsigned int rs, re;
> - size_t remains, n;
> + size_t remains = count, n;
>
> /*
> * If it's area created by vm_map_ram() interface directly, but
> @@ -3545,7 +3545,6 @@ static size_t vmap_ram_vread_iter(struct iov_iter *iter, const char *addr,
> goto finished_zero;
> }
>
> - remains = count;
> for_each_set_bitrange(rs, re, vb->used_map, VMAP_BBMAP_BITS) {
> size_t copied;
>
> --
> 2.39.2
>
>
This was also already fixed in a more recent version of this patch set, in
v5 I believe. Again apologies for this!
Latest version at
https://lore.kernel.org/all/cover.1679566220.git.lstoakes@gmail.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] mm: vmalloc: mark zero_iter() static
2023-03-23 11:58 [PATCH 1/3] mm/mmap/vma_merge: actually set next to NULL if not applicable Arnd Bergmann
2023-03-23 11:58 ` [PATCH 2/3] mm: vmalloc: fix vmap_ram_vread_iter() return value Arnd Bergmann
@ 2023-03-23 11:58 ` Arnd Bergmann
2023-03-23 12:06 ` Lorenzo Stoakes
2023-03-23 12:03 ` [PATCH 1/3] mm/mmap/vma_merge: actually set next to NULL if not applicable Lorenzo Stoakes
2 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2023-03-23 11:58 UTC (permalink / raw)
To: Andrew Morton, Lorenzo Stoakes
Cc: Arnd Bergmann, Uladzislau Rezki, Christoph Hellwig, Baoquan He,
David Hildenbrand, Andrey Konovalov, linux-mm, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
The zero_iter() function was put into the global namespace,
apparently by accident:
mm/vmalloc.c:3448:8: error: no previous prototype for function 'zero_iter' [-Werror,-Wmissing-prototypes]
size_t zero_iter(struct iov_iter *iter, size_t count)
^
mm/vmalloc.c:3448:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
size_t zero_iter(struct iov_iter *iter, size_t count)
^
Fixes: d9cab54f7737 ("mm: vmalloc: convert vread() to vread_iter()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
mm/vmalloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index fb216495fe5a..55f5d6fc8629 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -3445,7 +3445,7 @@ EXPORT_SYMBOL(vmalloc_32_user);
*
* Returns the number of zeroed bytes.
*/
-size_t zero_iter(struct iov_iter *iter, size_t count)
+static size_t zero_iter(struct iov_iter *iter, size_t count)
{
size_t remains = count;
--
2.39.2
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] mm: vmalloc: mark zero_iter() static
2023-03-23 11:58 ` [PATCH 3/3] mm: vmalloc: mark zero_iter() static Arnd Bergmann
@ 2023-03-23 12:06 ` Lorenzo Stoakes
0 siblings, 0 replies; 6+ messages in thread
From: Lorenzo Stoakes @ 2023-03-23 12:06 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Andrew Morton, Arnd Bergmann, Uladzislau Rezki,
Christoph Hellwig, Baoquan He, David Hildenbrand,
Andrey Konovalov, linux-mm, linux-kernel
On Thu, Mar 23, 2023 at 12:58:36PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The zero_iter() function was put into the global namespace,
> apparently by accident:
>
> mm/vmalloc.c:3448:8: error: no previous prototype for function 'zero_iter' [-Werror,-Wmissing-prototypes]
> size_t zero_iter(struct iov_iter *iter, size_t count)
> ^
> mm/vmalloc.c:3448:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> size_t zero_iter(struct iov_iter *iter, size_t count)
> ^
>
> Fixes: d9cab54f7737 ("mm: vmalloc: convert vread() to vread_iter()")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> mm/vmalloc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index fb216495fe5a..55f5d6fc8629 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -3445,7 +3445,7 @@ EXPORT_SYMBOL(vmalloc_32_user);
> *
> * Returns the number of zeroed bytes.
> */
> -size_t zero_iter(struct iov_iter *iter, size_t count)
> +static size_t zero_iter(struct iov_iter *iter, size_t count)
> {
> size_t remains = count;
>
> --
> 2.39.2
>
>
And finally, this was also fixed in the latest version (v8) see
https://lore.kernel.org/all/cover.1679566220.git.lstoakes@gmail.com
Again do forgive the noise! :)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] mm/mmap/vma_merge: actually set next to NULL if not applicable
2023-03-23 11:58 [PATCH 1/3] mm/mmap/vma_merge: actually set next to NULL if not applicable Arnd Bergmann
2023-03-23 11:58 ` [PATCH 2/3] mm: vmalloc: fix vmap_ram_vread_iter() return value Arnd Bergmann
2023-03-23 11:58 ` [PATCH 3/3] mm: vmalloc: mark zero_iter() static Arnd Bergmann
@ 2023-03-23 12:03 ` Lorenzo Stoakes
2 siblings, 0 replies; 6+ messages in thread
From: Lorenzo Stoakes @ 2023-03-23 12:03 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Andrew Morton, Arnd Bergmann, Nathan Chancellor,
Nick Desaulniers, Tom Rix, Liam R. Howlett, Vlastimil Babka,
Suren Baghdasaryan, linux-mm, linux-kernel, llvm
On Thu, Mar 23, 2023 at 12:58:34PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> As clang builds point out, the variable 'next' is now uninitialized
> in some conditions as a result of a previous patch that tried to
> rely on it being NULL here:
>
> mm/mmap.c:939:11: error: variable 'next' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
> else if (!curr)
> ^~~~~
> mm/mmap.c:952:15: note: uninitialized use occurs here
> merge_next = next && mpol_equal(policy, vma_policy(next)) &&
> ^~~~
>
> Fixes: e887ecae997e ("mm/mmap/vma_merge: set next to NULL if not applicable")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> mm/mmap.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 54099a604cf8..c01d43bd694e 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -939,6 +939,8 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm,
> else if (!curr)
> /* Is there a VMA next to a hole (case 1 - 3) or prev (4)? */
> next = vma_lookup(mm, end);
> + else
> + next = NULL;
>
> /* Can we merge the predecessor? */
> if (prev && addr == prev->vm_end && mpol_equal(vma_policy(prev), policy)
> --
> 2.39.2
>
This was already fixed in a more recent series of this patch set (at
v3). Sorry for this reaching -next!
See https://lore.kernel.org/all/cover.1679516210.git.lstoakes@gmail.com/
^ permalink raw reply [flat|nested] 6+ messages in thread