| 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
| ||
|
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