linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] mm/mmap/vma_merge: actually set next to NULL if not applicable
@ 2023-03-23 11:58 Arnd Bergmann
  2023-03-23 11:58 ` [PATCH 2/3] mm: vmalloc: fix vmap_ram_vread_iter() return value Arnd Bergmann
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Arnd Bergmann @ 2023-03-23 11:58 UTC (permalink / raw)
  To: Andrew Morton, Lorenzo Stoakes
  Cc: Arnd Bergmann, Nathan Chancellor, Nick Desaulniers, Tom Rix,
	Liam R. Howlett, Vlastimil Babka, Suren Baghdasaryan, linux-mm,
	linux-kernel, llvm

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [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 related	[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 related	[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

* 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

* 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

end of thread, other threads:[~2023-03-23 12:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 12:05   ` Lorenzo Stoakes
2023-03-23 11:58 ` [PATCH 3/3] mm: vmalloc: mark zero_iter() static 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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).