lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YoQeZIl2P9wOK+u8@yury-laptop>
Date:   Tue, 17 May 2022 15:15:00 -0700
From:   Yury Norov <yury.norov@...il.com>
To:     Kees Cook <keescook@...omium.org>
Cc:     Christophe de Dinechin <dinechin@...hat.com>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Zhen Lei <thunder.leizhen@...wei.com>,
        linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2 2/2] nodemask: Fix return values to be unsigned

On Tue, May 17, 2022 at 02:22:34PM -0700, Kees Cook wrote:
> The nodemask routines had mixed return values that provided potentially
> signed return values that could never happen. This was leading to the
> compiler getting confusing about the range of possible return values
> (it was thinking things could be negative where they could not be). Fix
> all the nodemask routines that should be returning unsigned
> (or bool) values. Silences:
> 
>  mm/swapfile.c: In function ‘setup_swap_info’:
>  mm/swapfile.c:2291:47: error: array subscript -1 is below array bounds of ‘struct plist_node[]’ [-Werror=array-bounds]
>   2291 |                                 p->avail_lists[i].prio = 1;
>        |                                 ~~~~~~~~~~~~~~^~~
>  In file included from mm/swapfile.c:16:
>  ./include/linux/swap.h:292:27: note: while referencing ‘avail_lists’
>    292 |         struct plist_node avail_lists[]; /*
>        |                           ^~~~~~~~~~~
> 
> Reported-by: Christophe de Dinechin <dinechin@...hat.com>
> Link: https://lore.kernel.org/lkml/20220414150855.2407137-3-dinechin@redhat.com/
> Cc: Alexey Dobriyan <adobriyan@...il.com>
> Cc: Yury Norov <yury.norov@...il.com>
> Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Cc: Rasmus Villemoes <linux@...musvillemoes.dk>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: Zhen Lei <thunder.leizhen@...wei.com>
> Signed-off-by: Kees Cook <keescook@...omium.org>
> ---
>  include/linux/nodemask.h | 38 +++++++++++++++++++-------------------
>  lib/nodemask.c           |  2 +-
>  2 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/include/linux/nodemask.h b/include/linux/nodemask.h
> index 567c3ddba2c4..2c39663c3407 100644
> --- a/include/linux/nodemask.h
> +++ b/include/linux/nodemask.h
> @@ -42,11 +42,11 @@
>   * void nodes_shift_right(dst, src, n)	Shift right
>   * void nodes_shift_left(dst, src, n)	Shift left
>   *
> - * int first_node(mask)			Number lowest set bit, or MAX_NUMNODES
> - * int next_node(node, mask)		Next node past 'node', or MAX_NUMNODES
> - * int next_node_in(node, mask)		Next node past 'node', or wrap to first,
> + * unsigned int first_node(mask)	Number lowest set bit, or MAX_NUMNODES
> + * unsigend int next_node(node, mask)	Next node past 'node', or MAX_NUMNODES
> + * unsigned int next_node_in(node, mask) Next node past 'node', or wrap to first,
>   *					or MAX_NUMNODES
> - * int first_unset_node(mask)		First node not set in mask, or 
> + * unsigned int first_unset_node(mask)	First node not set in mask, or
>   *					MAX_NUMNODES
>   *
>   * nodemask_t nodemask_of_node(node)	Return nodemask with bit 'node' set
> @@ -153,7 +153,7 @@ static inline void __nodes_clear(nodemask_t *dstp, unsigned int nbits)
>  
>  #define node_test_and_set(node, nodemask) \
>  			__node_test_and_set((node), &(nodemask))
> -static inline int __node_test_and_set(int node, nodemask_t *addr)
> +static inline bool __node_test_and_set(int node, nodemask_t *addr)
>  {
>  	return test_and_set_bit(node, addr->bits);
>  }
> @@ -200,7 +200,7 @@ static inline void __nodes_complement(nodemask_t *dstp,
>  
>  #define nodes_equal(src1, src2) \
>  			__nodes_equal(&(src1), &(src2), MAX_NUMNODES)
> -static inline int __nodes_equal(const nodemask_t *src1p,
> +static inline bool __nodes_equal(const nodemask_t *src1p,
>  					const nodemask_t *src2p, unsigned int nbits)
>  {
>  	return bitmap_equal(src1p->bits, src2p->bits, nbits);
> @@ -208,7 +208,7 @@ static inline int __nodes_equal(const nodemask_t *src1p,
>  
>  #define nodes_intersects(src1, src2) \
>  			__nodes_intersects(&(src1), &(src2), MAX_NUMNODES)
> -static inline int __nodes_intersects(const nodemask_t *src1p,
> +static inline bool __nodes_intersects(const nodemask_t *src1p,
>  					const nodemask_t *src2p, unsigned int nbits)
>  {
>  	return bitmap_intersects(src1p->bits, src2p->bits, nbits);
> @@ -216,20 +216,20 @@ static inline int __nodes_intersects(const nodemask_t *src1p,
>  
>  #define nodes_subset(src1, src2) \
>  			__nodes_subset(&(src1), &(src2), MAX_NUMNODES)
> -static inline int __nodes_subset(const nodemask_t *src1p,
> +static inline bool __nodes_subset(const nodemask_t *src1p,
>  					const nodemask_t *src2p, unsigned int nbits)
>  {
>  	return bitmap_subset(src1p->bits, src2p->bits, nbits);
>  }
>  
>  #define nodes_empty(src) __nodes_empty(&(src), MAX_NUMNODES)
> -static inline int __nodes_empty(const nodemask_t *srcp, unsigned int nbits)
> +static inline bool __nodes_empty(const nodemask_t *srcp, unsigned int nbits)
>  {
>  	return bitmap_empty(srcp->bits, nbits);
>  }
>  
>  #define nodes_full(nodemask) __nodes_full(&(nodemask), MAX_NUMNODES)
> -static inline int __nodes_full(const nodemask_t *srcp, unsigned int nbits)
> +static inline bool __nodes_full(const nodemask_t *srcp, unsigned int nbits)
>  {
>  	return bitmap_full(srcp->bits, nbits);
>  }
> @@ -260,15 +260,15 @@ static inline void __nodes_shift_left(nodemask_t *dstp,
>            > MAX_NUMNODES, then the silly min_ts could be dropped. */
>  
>  #define first_node(src) __first_node(&(src))
> -static inline int __first_node(const nodemask_t *srcp)
> +static inline unsigned int __first_node(const nodemask_t *srcp)
>  {
> -	return min_t(int, MAX_NUMNODES, find_first_bit(srcp->bits, MAX_NUMNODES));
> +	return min_t(unsigned int, MAX_NUMNODES, find_first_bit(srcp->bits, MAX_NUMNODES));
>  }
>  
>  #define next_node(n, src) __next_node((n), &(src))
> -static inline int __next_node(int n, const nodemask_t *srcp)
> +static inline unsigned int __next_node(int n, const nodemask_t *srcp)
>  {
> -	return min_t(int,MAX_NUMNODES,find_next_bit(srcp->bits, MAX_NUMNODES, n+1));
> +	return min_t(unsigned int, MAX_NUMNODES, find_next_bit(srcp->bits, MAX_NUMNODES, n+1));
>  }
>  
>  /*
> @@ -276,7 +276,7 @@ static inline int __next_node(int n, const nodemask_t *srcp)
>   * the first node in src if needed.  Returns MAX_NUMNODES if src is empty.
>   */
>  #define next_node_in(n, src) __next_node_in((n), &(src))
> -int __next_node_in(int node, const nodemask_t *srcp);
> +unsigned int __next_node_in(int node, const nodemask_t *srcp);
>  
>  static inline void init_nodemask_of_node(nodemask_t *mask, int node)
>  {
> @@ -296,9 +296,9 @@ static inline void init_nodemask_of_node(nodemask_t *mask, int node)
>  })
>  
>  #define first_unset_node(mask) __first_unset_node(&(mask))
> -static inline int __first_unset_node(const nodemask_t *maskp)
> +static inline unsigned int __first_unset_node(const nodemask_t *maskp)
>  {
> -	return min_t(int,MAX_NUMNODES,
> +	return min_t(unsigned int, MAX_NUMNODES,
>  			find_first_zero_bit(maskp->bits, MAX_NUMNODES));
>  }
>  
> @@ -436,11 +436,11 @@ static inline int num_node_state(enum node_states state)
>  
>  #define first_online_node	first_node(node_states[N_ONLINE])
>  #define first_memory_node	first_node(node_states[N_MEMORY])
> -static inline int next_online_node(int nid)
> +static inline unsigned int next_online_node(int nid)
>  {
>  	return next_node(nid, node_states[N_ONLINE]);
>  }
> -static inline int next_memory_node(int nid)
> +static inline unsigned int next_memory_node(int nid)
>  {
>  	return next_node(nid, node_states[N_MEMORY]);
>  }
> diff --git a/lib/nodemask.c b/lib/nodemask.c
> index 3aa454c54c0d..a334cf722189 100644
> --- a/lib/nodemask.c
> +++ b/lib/nodemask.c
> @@ -3,7 +3,7 @@
>  #include <linux/module.h>
>  #include <linux/random.h>
>  
> -int __next_node_in(int node, const nodemask_t *srcp)
> +unsigned int __next_node_in(int node, const nodemask_t *srcp)
>  {
>  	int ret = __next_node(node, srcp);

Now this should be unsigned int, right?
  
> -- 
> 2.32.0

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ