[PATCH kcsan 0/3] KCSAN updates for 5.11

[PATCH kcsan 0/3] KCSAN updates for 5.11

[Paul E. McKenney]

Hello!

This series provides KCSAN updates:

1.	Ensure that selftest address is at least PAGE_SIZE.

2.	Never set up watchpoints on NULL pointers.

3.	Fix encoding masks and regain address bit.

						Thanx, Paul

------------------------------------------------------------------------

 encoding.h |   20 +++++++++++---------
 selftest.c |    3 +++
 2 files changed, 14 insertions(+), 9 deletions(-)

[PATCH kcsan 1/3] kcsan: selftest: Ensure that address is at least PAGE_SIZE

[paulmck]

From: Marco Elver <elver@google.com>

In preparation of supporting only addresses not within the NULL page,
change the selftest to never use addresses that are less than PAGE_SIZE.

Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Marco Elver <elver@google.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 kernel/kcsan/selftest.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/kcsan/selftest.c b/kernel/kcsan/selftest.c
index d98bc20..9014a3a 100644
--- a/kernel/kcsan/selftest.c
+++ b/kernel/kcsan/selftest.c
@@ -33,6 +33,9 @@ static bool test_encode_decode(void)
 		unsigned long addr;
 
 		prandom_bytes(&addr, sizeof(addr));
+		if (addr < PAGE_SIZE)
+			addr = PAGE_SIZE;
+
 		if (WARN_ON(!check_encodable(addr, size)))
 			return false;
 
-- 
2.9.5

[PATCH kcsan 2/3] kcsan: Never set up watchpoints on NULL pointers

[paulmck]

From: Marco Elver <elver@google.com>

Avoid setting up watchpoints on NULL pointers, as otherwise we would
crash inside the KCSAN runtime (when checking for value changes) instead
of the instrumented code.

Because that may be confusing, skip any address less than PAGE_SIZE.

Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Marco Elver <elver@google.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 kernel/kcsan/encoding.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/kcsan/encoding.h b/kernel/kcsan/encoding.h
index 1a6db2f..4f73db6 100644
--- a/kernel/kcsan/encoding.h
+++ b/kernel/kcsan/encoding.h
@@ -48,7 +48,11 @@
 
 static inline bool check_encodable(unsigned long addr, size_t size)
 {
-	return size <= MAX_ENCODABLE_SIZE;
+	/*
+	 * While we can encode addrs<PAGE_SIZE, avoid crashing with a NULL
+	 * pointer deref inside KCSAN.
+	 */
+	return addr >= PAGE_SIZE && size <= MAX_ENCODABLE_SIZE;
 }
 
 static inline long
-- 
2.9.5

[PATCH kcsan 3/3] kcsan: Fix encoding masks and regain address bit

[paulmck]

From: Marco Elver <elver@google.com>

The watchpoint encoding masks for size and address were off-by-one bit
each, with the size mask using 1 unnecessary bit and the address mask
missing 1 bit. However, due to the way the size is shifted into the
encoded watchpoint, we were effectively wasting and never using the
extra bit.

For example, on x86 with PAGE_SIZE==4K, we have 1 bit for the is-write
bit, 14 bits for the size bits, and then 49 bits left for the address.
Prior to this fix we would end up with this usage:

	[ write<1> | size<14> | wasted<1> | address<48> ]

Fix it by subtracting 1 bit from the GENMASK() end and start ranges of
size and address respectively. The added static_assert()s verify that
the masks are as expected. With the fixed version, we get the expected
usage:

	[ write<1> | size<14> |             address<49> ]

Functionally no change is expected, since that extra address bit is
insignificant for enabled architectures.

Signed-off-by: Marco Elver <elver@google.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 kernel/kcsan/encoding.h | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/kernel/kcsan/encoding.h b/kernel/kcsan/encoding.h
index 4f73db6..b50bda9 100644
--- a/kernel/kcsan/encoding.h
+++ b/kernel/kcsan/encoding.h
@@ -37,14 +37,12 @@
  */
 #define WATCHPOINT_ADDR_BITS (BITS_PER_LONG-1 - WATCHPOINT_SIZE_BITS)
 
-/*
- * Masks to set/retrieve the encoded data.
- */
-#define WATCHPOINT_WRITE_MASK BIT(BITS_PER_LONG-1)
-#define WATCHPOINT_SIZE_MASK                                                   \
-	GENMASK(BITS_PER_LONG-2, BITS_PER_LONG-2 - WATCHPOINT_SIZE_BITS)
-#define WATCHPOINT_ADDR_MASK                                                   \
-	GENMASK(BITS_PER_LONG-3 - WATCHPOINT_SIZE_BITS, 0)
+/* Bitmasks for the encoded watchpoint access information. */
+#define WATCHPOINT_WRITE_MASK	BIT(BITS_PER_LONG-1)
+#define WATCHPOINT_SIZE_MASK	GENMASK(BITS_PER_LONG-2, BITS_PER_LONG-1 - WATCHPOINT_SIZE_BITS)
+#define WATCHPOINT_ADDR_MASK	GENMASK(BITS_PER_LONG-2 - WATCHPOINT_SIZE_BITS, 0)
+static_assert(WATCHPOINT_ADDR_MASK == (1UL << WATCHPOINT_ADDR_BITS) - 1);
+static_assert((WATCHPOINT_WRITE_MASK ^ WATCHPOINT_SIZE_MASK ^ WATCHPOINT_ADDR_MASK) == ~0UL);
 
 static inline bool check_encodable(unsigned long addr, size_t size)
 {
-- 
2.9.5

Re: [PATCH kcsan 3/3] kcsan: Fix encoding masks and regain address bit

[Boqun Feng]

Hi Marco,

On Thu, Nov 05, 2020 at 02:03:24PM -0800, paulmck@kernel.org wrote:
> From: Marco Elver <elver@google.com>
> 
> The watchpoint encoding masks for size and address were off-by-one bit
> each, with the size mask using 1 unnecessary bit and the address mask
> missing 1 bit. However, due to the way the size is shifted into the
> encoded watchpoint, we were effectively wasting and never using the
> extra bit.
> 
> For example, on x86 with PAGE_SIZE==4K, we have 1 bit for the is-write
> bit, 14 bits for the size bits, and then 49 bits left for the address.
> Prior to this fix we would end up with this usage:
> 
> 	[ write<1> | size<14> | wasted<1> | address<48> ]
> 
> Fix it by subtracting 1 bit from the GENMASK() end and start ranges of
> size and address respectively. The added static_assert()s verify that
> the masks are as expected. With the fixed version, we get the expected
> usage:
> 
> 	[ write<1> | size<14> |             address<49> ]
> 
> Functionally no change is expected, since that extra address bit is
> insignificant for enabled architectures.
> 
> Signed-off-by: Marco Elver <elver@google.com>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> ---
>  kernel/kcsan/encoding.h | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/kcsan/encoding.h b/kernel/kcsan/encoding.h
> index 4f73db6..b50bda9 100644
> --- a/kernel/kcsan/encoding.h
> +++ b/kernel/kcsan/encoding.h
> @@ -37,14 +37,12 @@
>   */
>  #define WATCHPOINT_ADDR_BITS (BITS_PER_LONG-1 - WATCHPOINT_SIZE_BITS)
>  
> -/*
> - * Masks to set/retrieve the encoded data.
> - */
> -#define WATCHPOINT_WRITE_MASK BIT(BITS_PER_LONG-1)
> -#define WATCHPOINT_SIZE_MASK                                                   \
> -	GENMASK(BITS_PER_LONG-2, BITS_PER_LONG-2 - WATCHPOINT_SIZE_BITS)
> -#define WATCHPOINT_ADDR_MASK                                                   \
> -	GENMASK(BITS_PER_LONG-3 - WATCHPOINT_SIZE_BITS, 0)
> +/* Bitmasks for the encoded watchpoint access information. */
> +#define WATCHPOINT_WRITE_MASK	BIT(BITS_PER_LONG-1)
> +#define WATCHPOINT_SIZE_MASK	GENMASK(BITS_PER_LONG-2, BITS_PER_LONG-1 - WATCHPOINT_SIZE_BITS)
> +#define WATCHPOINT_ADDR_MASK	GENMASK(BITS_PER_LONG-2 - WATCHPOINT_SIZE_BITS, 0)
> +static_assert(WATCHPOINT_ADDR_MASK == (1UL << WATCHPOINT_ADDR_BITS) - 1);

Nit:

Since you use the static_assert(), why not define WATCHPOINT_ADDR_MASK
as:

#define WATCHPOINT_ADDR_MASK (BIT(WATCHPOINT_SIZE_BITS) - 1)

Besides, WATCHPOINT_SIZE_MASK can also be defined as:

#define WATCHPOINT_SIZE_MASK GENMASK(BITS_PER_LONG - 2, WATCHPOINT_SIZE_BITS)

Regards,
Boqun

> +static_assert((WATCHPOINT_WRITE_MASK ^ WATCHPOINT_SIZE_MASK ^ WATCHPOINT_ADDR_MASK) == ~0UL);
>  
>  static inline bool check_encodable(unsigned long addr, size_t size)
>  {
> -- 
> 2.9.5
> 

Re: [PATCH kcsan 3/3] kcsan: Fix encoding masks and regain address bit

[Marco Elver]

On Fri, 6 Nov 2020 at 02:23, Boqun Feng <boqun.feng@gmail.com> wrote:
> Hi Marco,
>
> On Thu, Nov 05, 2020 at 02:03:24PM -0800, paulmck@kernel.org wrote:
> > From: Marco Elver <elver@google.com>
> >
> > The watchpoint encoding masks for size and address were off-by-one bit
> > each, with the size mask using 1 unnecessary bit and the address mask
> > missing 1 bit. However, due to the way the size is shifted into the
> > encoded watchpoint, we were effectively wasting and never using the
> > extra bit.
> >
> > For example, on x86 with PAGE_SIZE==4K, we have 1 bit for the is-write
> > bit, 14 bits for the size bits, and then 49 bits left for the address.
> > Prior to this fix we would end up with this usage:
> >
> >       [ write<1> | size<14> | wasted<1> | address<48> ]
> >
> > Fix it by subtracting 1 bit from the GENMASK() end and start ranges of
> > size and address respectively. The added static_assert()s verify that
> > the masks are as expected. With the fixed version, we get the expected
> > usage:
> >
> >       [ write<1> | size<14> |             address<49> ]
> >
> > Functionally no change is expected, since that extra address bit is
> > insignificant for enabled architectures.
> >
> > Signed-off-by: Marco Elver <elver@google.com>
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > ---
> >  kernel/kcsan/encoding.h | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/kernel/kcsan/encoding.h b/kernel/kcsan/encoding.h
> > index 4f73db6..b50bda9 100644
> > --- a/kernel/kcsan/encoding.h
> > +++ b/kernel/kcsan/encoding.h
> > @@ -37,14 +37,12 @@
> >   */
> >  #define WATCHPOINT_ADDR_BITS (BITS_PER_LONG-1 - WATCHPOINT_SIZE_BITS)
> >
> > -/*
> > - * Masks to set/retrieve the encoded data.
> > - */
> > -#define WATCHPOINT_WRITE_MASK BIT(BITS_PER_LONG-1)
> > -#define WATCHPOINT_SIZE_MASK                                                   \
> > -     GENMASK(BITS_PER_LONG-2, BITS_PER_LONG-2 - WATCHPOINT_SIZE_BITS)
> > -#define WATCHPOINT_ADDR_MASK                                                   \
> > -     GENMASK(BITS_PER_LONG-3 - WATCHPOINT_SIZE_BITS, 0)
> > +/* Bitmasks for the encoded watchpoint access information. */
> > +#define WATCHPOINT_WRITE_MASK        BIT(BITS_PER_LONG-1)
> > +#define WATCHPOINT_SIZE_MASK GENMASK(BITS_PER_LONG-2, BITS_PER_LONG-1 - WATCHPOINT_SIZE_BITS)
> > +#define WATCHPOINT_ADDR_MASK GENMASK(BITS_PER_LONG-2 - WATCHPOINT_SIZE_BITS, 0)
> > +static_assert(WATCHPOINT_ADDR_MASK == (1UL << WATCHPOINT_ADDR_BITS) - 1);
>
> Nit:
>
> Since you use the static_assert(), why not define WATCHPOINT_ADDR_MASK
> as:
>
> #define WATCHPOINT_ADDR_MASK (BIT(WATCHPOINT_SIZE_BITS) - 1)

This is incorrect, as the static_assert()s would have indicated. It
should probably be (BIT(WATCHPOINT_ADDR_BITS) - 1)?

As an aside, I explicitly did *not* want to use additional arithmetic
to generate the masks but purely rely on BIT(), and GENMASK(), as it
would be inconsistent otherwise. The static_assert()s then sanity
check everything without BIT+GENMASK (because I've grown slightly
paranoid about off-by-1s here). So I'd rather not start bikeshedding
about which way around things should go.

In general, GENMASK() is safer, because subtracting 1 to get the mask
doesn't always work, specifically e.g. (BIT(BITS_PER_LONG) - 1) does
not work.

> Besides, WATCHPOINT_SIZE_MASK can also be defined as:

No, sorry it cannot.

> #define WATCHPOINT_SIZE_MASK GENMASK(BITS_PER_LONG - 2, WATCHPOINT_SIZE_BITS)

   GENMASK(BITS_PER_LONG - 2, WATCHPOINT_SIZE_BITS)

is not equivalent to the current

  GENMASK(BITS_PER_LONG-2, BITS_PER_LONG-1 - WATCHPOINT_SIZE_BITS)

Did you mean GENMASK(BITS_PER_LONG-2, WATCHPOINT_ADDR_BITS)? I can
send a v2 for this one.

Thanks,
-- Marco

[PATCH v2] kcsan: Fix encoding masks and regain address bit

[Marco Elver]

The watchpoint encoding masks for size and address were off-by-one bit
each, with the size mask using 1 unnecessary bit and the address mask
missing 1 bit. However, due to the way the size is shifted into the
encoded watchpoint, we were effectively wasting and never using the
extra bit.

For example, on x86 with PAGE_SIZE==4K, we have 1 bit for the is-write
bit, 14 bits for the size bits, and then 49 bits left for the address.
Prior to this fix we would end up with this usage:

	[ write<1> | size<14> | wasted<1> | address<48> ]

Fix it by subtracting 1 bit from the GENMASK() end and start ranges of
size and address respectively. The added static_assert()s verify that
the masks are as expected. With the fixed version, we get the expected
usage:

	[ write<1> | size<14> |             address<49> ]

Functionally no change is expected, since that extra address bit is
insignificant for enabled architectures.

Signed-off-by: Marco Elver <elver@google.com>
---
v2:
* Use WATCHPOINT_ADDR_BITS to avoid duplicating "BITS_PER_LONG-1 -
  WATCHPOINT_SIZE_BITS" per Boqun's suggestion.
---
 kernel/kcsan/encoding.h | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/kernel/kcsan/encoding.h b/kernel/kcsan/encoding.h
index 4f73db6d1407..7ee405524904 100644
--- a/kernel/kcsan/encoding.h
+++ b/kernel/kcsan/encoding.h
@@ -37,14 +37,12 @@
  */
 #define WATCHPOINT_ADDR_BITS (BITS_PER_LONG-1 - WATCHPOINT_SIZE_BITS)
 
-/*
- * Masks to set/retrieve the encoded data.
- */
-#define WATCHPOINT_WRITE_MASK BIT(BITS_PER_LONG-1)
-#define WATCHPOINT_SIZE_MASK                                                   \
-	GENMASK(BITS_PER_LONG-2, BITS_PER_LONG-2 - WATCHPOINT_SIZE_BITS)
-#define WATCHPOINT_ADDR_MASK                                                   \
-	GENMASK(BITS_PER_LONG-3 - WATCHPOINT_SIZE_BITS, 0)
+/* Bitmasks for the encoded watchpoint access information. */
+#define WATCHPOINT_WRITE_MASK	BIT(BITS_PER_LONG-1)
+#define WATCHPOINT_SIZE_MASK	GENMASK(BITS_PER_LONG-2, WATCHPOINT_ADDR_BITS)
+#define WATCHPOINT_ADDR_MASK	GENMASK(WATCHPOINT_ADDR_BITS-1, 0)
+static_assert(WATCHPOINT_ADDR_MASK == (1UL << WATCHPOINT_ADDR_BITS) - 1);
+static_assert((WATCHPOINT_WRITE_MASK ^ WATCHPOINT_SIZE_MASK ^ WATCHPOINT_ADDR_MASK) == ~0UL);
 
 static inline bool check_encodable(unsigned long addr, size_t size)
 {
-- 
2.29.2.222.g5d2a92d10f8-goog

Re: [PATCH kcsan 3/3] kcsan: Fix encoding masks and regain address bit

[Boqun Feng]

On Fri, Nov 06, 2020 at 10:03:21AM +0100, Marco Elver wrote:
> On Fri, 6 Nov 2020 at 02:23, Boqun Feng <boqun.feng@gmail.com> wrote:
> > Hi Marco,
> >
> > On Thu, Nov 05, 2020 at 02:03:24PM -0800, paulmck@kernel.org wrote:
> > > From: Marco Elver <elver@google.com>
> > >
> > > The watchpoint encoding masks for size and address were off-by-one bit
> > > each, with the size mask using 1 unnecessary bit and the address mask
> > > missing 1 bit. However, due to the way the size is shifted into the
> > > encoded watchpoint, we were effectively wasting and never using the
> > > extra bit.
> > >
> > > For example, on x86 with PAGE_SIZE==4K, we have 1 bit for the is-write
> > > bit, 14 bits for the size bits, and then 49 bits left for the address.
> > > Prior to this fix we would end up with this usage:
> > >
> > >       [ write<1> | size<14> | wasted<1> | address<48> ]
> > >
> > > Fix it by subtracting 1 bit from the GENMASK() end and start ranges of
> > > size and address respectively. The added static_assert()s verify that
> > > the masks are as expected. With the fixed version, we get the expected
> > > usage:
> > >
> > >       [ write<1> | size<14> |             address<49> ]
> > >
> > > Functionally no change is expected, since that extra address bit is
> > > insignificant for enabled architectures.
> > >
> > > Signed-off-by: Marco Elver <elver@google.com>
> > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > > ---
> > >  kernel/kcsan/encoding.h | 14 ++++++--------
> > >  1 file changed, 6 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/kernel/kcsan/encoding.h b/kernel/kcsan/encoding.h
> > > index 4f73db6..b50bda9 100644
> > > --- a/kernel/kcsan/encoding.h
> > > +++ b/kernel/kcsan/encoding.h
> > > @@ -37,14 +37,12 @@
> > >   */
> > >  #define WATCHPOINT_ADDR_BITS (BITS_PER_LONG-1 - WATCHPOINT_SIZE_BITS)
> > >
> > > -/*
> > > - * Masks to set/retrieve the encoded data.
> > > - */
> > > -#define WATCHPOINT_WRITE_MASK BIT(BITS_PER_LONG-1)
> > > -#define WATCHPOINT_SIZE_MASK                                                   \
> > > -     GENMASK(BITS_PER_LONG-2, BITS_PER_LONG-2 - WATCHPOINT_SIZE_BITS)
> > > -#define WATCHPOINT_ADDR_MASK                                                   \
> > > -     GENMASK(BITS_PER_LONG-3 - WATCHPOINT_SIZE_BITS, 0)
> > > +/* Bitmasks for the encoded watchpoint access information. */
> > > +#define WATCHPOINT_WRITE_MASK        BIT(BITS_PER_LONG-1)
> > > +#define WATCHPOINT_SIZE_MASK GENMASK(BITS_PER_LONG-2, BITS_PER_LONG-1 - WATCHPOINT_SIZE_BITS)
> > > +#define WATCHPOINT_ADDR_MASK GENMASK(BITS_PER_LONG-2 - WATCHPOINT_SIZE_BITS, 0)
> > > +static_assert(WATCHPOINT_ADDR_MASK == (1UL << WATCHPOINT_ADDR_BITS) - 1);
> >
> > Nit:
> >
> > Since you use the static_assert(), why not define WATCHPOINT_ADDR_MASK
> > as:
> >
> > #define WATCHPOINT_ADDR_MASK (BIT(WATCHPOINT_SIZE_BITS) - 1)
> 
> This is incorrect, as the static_assert()s would have indicated. It
> should probably be (BIT(WATCHPOINT_ADDR_BITS) - 1)?
> 
> As an aside, I explicitly did *not* want to use additional arithmetic
> to generate the masks but purely rely on BIT(), and GENMASK(), as it
> would be inconsistent otherwise. The static_assert()s then sanity
> check everything without BIT+GENMASK (because I've grown slightly
> paranoid about off-by-1s here). So I'd rather not start bikeshedding
> about which way around things should go.
> 
> In general, GENMASK() is safer, because subtracting 1 to get the mask
> doesn't always work, specifically e.g. (BIT(BITS_PER_LONG) - 1) does
> not work.
> 
> > Besides, WATCHPOINT_SIZE_MASK can also be defined as:
> 
> No, sorry it cannot.
> 
> > #define WATCHPOINT_SIZE_MASK GENMASK(BITS_PER_LONG - 2, WATCHPOINT_SIZE_BITS)
> 
>    GENMASK(BITS_PER_LONG - 2, WATCHPOINT_SIZE_BITS)
> 
> is not equivalent to the current
> 
>   GENMASK(BITS_PER_LONG-2, BITS_PER_LONG-1 - WATCHPOINT_SIZE_BITS)
> 
> Did you mean GENMASK(BITS_PER_LONG-2, WATCHPOINT_ADDR_BITS)? I can

You're right! Guess I should check first about what vim completes for me
;-) And I agree with you on the preference to GENMASK()

> send a v2 for this one.

Let me add an ack for that one, thanks!

Regards,
Boqun

> 
> Thanks,
> -- Marco

Re: [PATCH v2] kcsan: Fix encoding masks and regain address bit

[Boqun Feng]

On Fri, Nov 06, 2020 at 10:34:56AM +0100, Marco Elver wrote:
> The watchpoint encoding masks for size and address were off-by-one bit
> each, with the size mask using 1 unnecessary bit and the address mask
> missing 1 bit. However, due to the way the size is shifted into the
> encoded watchpoint, we were effectively wasting and never using the
> extra bit.
> 
> For example, on x86 with PAGE_SIZE==4K, we have 1 bit for the is-write
> bit, 14 bits for the size bits, and then 49 bits left for the address.
> Prior to this fix we would end up with this usage:
> 
> 	[ write<1> | size<14> | wasted<1> | address<48> ]
> 
> Fix it by subtracting 1 bit from the GENMASK() end and start ranges of
> size and address respectively. The added static_assert()s verify that
> the masks are as expected. With the fixed version, we get the expected
> usage:
> 
> 	[ write<1> | size<14> |             address<49> ]
> 
> Functionally no change is expected, since that extra address bit is
> insignificant for enabled architectures.
> 
> Signed-off-by: Marco Elver <elver@google.com>

Acked-by: Boqun Feng <boqun.feng@gmail.com>

Regards,
Boqun

> ---
> v2:
> * Use WATCHPOINT_ADDR_BITS to avoid duplicating "BITS_PER_LONG-1 -
>   WATCHPOINT_SIZE_BITS" per Boqun's suggestion.
> ---
>  kernel/kcsan/encoding.h | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/kcsan/encoding.h b/kernel/kcsan/encoding.h
> index 4f73db6d1407..7ee405524904 100644
> --- a/kernel/kcsan/encoding.h
> +++ b/kernel/kcsan/encoding.h
> @@ -37,14 +37,12 @@
>   */
>  #define WATCHPOINT_ADDR_BITS (BITS_PER_LONG-1 - WATCHPOINT_SIZE_BITS)
>  
> -/*
> - * Masks to set/retrieve the encoded data.
> - */
> -#define WATCHPOINT_WRITE_MASK BIT(BITS_PER_LONG-1)
> -#define WATCHPOINT_SIZE_MASK                                                   \
> -	GENMASK(BITS_PER_LONG-2, BITS_PER_LONG-2 - WATCHPOINT_SIZE_BITS)
> -#define WATCHPOINT_ADDR_MASK                                                   \
> -	GENMASK(BITS_PER_LONG-3 - WATCHPOINT_SIZE_BITS, 0)
> +/* Bitmasks for the encoded watchpoint access information. */
> +#define WATCHPOINT_WRITE_MASK	BIT(BITS_PER_LONG-1)
> +#define WATCHPOINT_SIZE_MASK	GENMASK(BITS_PER_LONG-2, WATCHPOINT_ADDR_BITS)
> +#define WATCHPOINT_ADDR_MASK	GENMASK(WATCHPOINT_ADDR_BITS-1, 0)
> +static_assert(WATCHPOINT_ADDR_MASK == (1UL << WATCHPOINT_ADDR_BITS) - 1);
> +static_assert((WATCHPOINT_WRITE_MASK ^ WATCHPOINT_SIZE_MASK ^ WATCHPOINT_ADDR_MASK) == ~0UL);
>  
>  static inline bool check_encodable(unsigned long addr, size_t size)
>  {
> -- 
> 2.29.2.222.g5d2a92d10f8-goog
> 

Re: [PATCH kcsan 3/3] kcsan: Fix encoding masks and regain address bit

[Marco Elver]

On Fri, 6 Nov 2020 at 11:19, Boqun Feng <boqun.feng@gmail.com> wrote:

> > send a v2 for this one.
>
> Let me add an ack for that one, thanks!

Thank you!

-- Marco