re: NetBSD Kernel Sanitizers

[matthew green <mrg%eterna23.net@localhost>]

thanks for looking at this.

> In function 'be16enc',
>     inlined from 'uuid_enc_be' at /home/palle/netbsd/git/src/sys/kern/kern_uuid.c:178:2:
>     /home/palle/netbsd/git/src/sys/sys/endian.h:204:9: error: '__builtin_memcpy' offset [0, 1] is out of the bounds [0, 0] [-Werror=array-bounds]
>       204 |         __builtin_memcpy(dst, &u, sizeof(u)); \
> 	    |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 	    /home/palle/netbsd/git/src/sys/sys/endian.h:207:1: note: in expansion of macro '__GEN_ENDIAN_ENC'
> 	      207 | __GEN_ENDIAN_ENC(16, be)
> 		    | ^~~~~~~~~~~~~~~~
> 		    In function 'be16dec',

this seems to be a valid complaint, in at least one sense.

uuid_enc_be(void *buf, const struct uuid *uuid)
{
        uint8_t *p = buf;
...

        be32enc(p, uuid->time_low);
        be16enc(p + 4, uuid->time_mid);
        be16enc(p + 6, uuid->time_hi_and_version);

in these cases, "p" technically only points to one byte, so all
of them in some sense trigger the failure.

can you see if adding (uint32 *) for 32 bit, and (uint16 *) for
16 bit calls here, if that avoids the warning?

> evbppc:
>
> In file included from /home/palle/netbsd/git/src/sys/dev/rasops/rasops2.c:49:
> /home/palle/netbsd/git/src/sys/dev/rasops/rasops_bitops.h: In function 'rasops2_copycols':
> /home/palle/netbsd/git/src/sys/dev/rasops/rasops_masks.h:85:50: error: array subscript 2147483616 is above array bounds of 'const uint32_t[33]' {aka 'const u
> nsigned int[33]'} [-Werror=array-bounds]
>    85 |                 (dp)[1] = ((dp)[1] & rasops_rmask[n]) |                 \
> 	 |                                      ~~~~~~~~~~~~^~~
> 	 /home/palle/netbsd/git/src/sys/dev/rasops/rasops_bitops.h:279:25: note: in expansion of macro 'PUTBITS'
> 	   279 |                         PUTBITS(tmp, db, num, drp);
> 		 |                         ^~~~~~~
> 		 /home/palle/netbsd/git/src/sys/dev/rasops/rasops_masks.h:92:25: note: while referencing 'rasops_rmask'
> 		    92 | extern const uint32_t   rasops_rmask[32 + 1];
> 			  |                         ^~~~~~~~~~~~
> 			  /home/palle/netbsd/git/src/sys/dev/rasops/rasops_masks.h:86:56: error: array subscript 2147483616 is above array bounds of 'const uin
> t32_t[33]' {aka 'const unsigned int[33]'} [-Werror=array-bounds]
> 			     86 |                         (MBL(sw, 32-(x)) & rasops_lmask[n]);            \
> 				   |                                            ~~~~~~~~~~~~^~~
> 				   /home/palle/netbsd/git/src/sys/dev/rasops/rasops_bitops.h:279:25: note: in expansion of macro 'PUTBITS'
> 				     279 |                         PUTBITS(tmp, db, num, drp);
> 					   |                         ^~~~~~~
> 					   /home/palle/netbsd/git/src/sys/dev/rasops/rasops_masks.h:91:25: note: while referencing 'rasops_lmask'
> 					      91 | extern const uint32_t   rasops_lmask[32 + 1];
> 						    |                         ^~~~~~~~~~~~

for this one, can you try making "unsigned num" here?

sys/dev/rasops/rasops_bitops.h:NAME(copycols)(void *cookie, int row, int src, int dst, int num)

and fixing anything that complains?  "num" here is the input triggering
2147483616 here..

> evbmips64-el:
>
> -- kern-MALTA32 ---
> --- aes_bear.o ---
> In function 'le32dec',
>     inlined from 'aesbear_cbc_dec.part.0' at /home/palle/netbsd/git/src/sys/crypto/aes/aes_bear.c:236:8:
>     /home/palle/netbsd/git/src/sys/sys/endian.h:220:9: error: '__builtin_memcpy' offset [0, 3] is out of the bounds [0, 0] [-Werror=array-bounds]
>       220 |         __builtin_memcpy(&u, buf, sizeof(u)); \
> 	    |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 	    /home/palle/netbsd/git/src/sys/sys/endian.h:228:1: note: in expansion of macro '__GEN_ENDIAN_DEC'
> 	      228 | __GEN_ENDIAN_DEC(32, le)
> 		    | ^~~~~~~~~~~~~~~~
> 		    --

this one is like the kern_uuid.c one, except harder to fix:

aesbear_cbc_dec(const struct aesdec *dec, const uint8_t in[static 16],
    uint8_t out[static 16], size_t nbytes, uint8_t iv[static 16],
...
        iv1 = le32dec(iv + 4*1);

this one seems worse than the prior one because it's obviously
valid because the input can't read-overflow.

probably fixable with similar annoying cast.

> evbmips64-eb:
>
> --- kern-MALTA32 ---
> In function 'be16dec',
>     inlined from 'uuid_dec_be' at /home/palle/netbsd/git/src/sys/kern/kern_uuid.c:193:19:

same as sparc64?

> riscv32:
> riscv64:
>
> bmake[2]: stopped making "all" in /home/palle/netbsd/build/riscv32/objdir/sys/arch/riscv/compile/GENERIC
> --- chacha_ref.o ---
> In function 'le32dec',
>     inlined from 'chacha_stream_ref' at /home/palle/netbsd/git/src/sys/crypto/chacha/chacha_ref.c:138:7:
>     /home/palle/netbsd/git/src/sys/sys/endian.h:220:9: error: '__builtin_memcpy' offset [0, 3] is out of the bounds [0, 0] [-Werror=array-bounds]
>       220 |         __builtin_memcpy(&u, buf, sizeof(u)); \
> 	    |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 	    /home/palle/netbsd/git/src/sys/sys/endian.h:228:1: note: in expansion of macro '__GEN_ENDIAN_DEC'
> 	      228 | __GEN_ENDIAN_DEC(32, le)
> 		    | ^~~~~~~~~~~~~~~~

same again as kern_uuid.c / aes_bear.c.

> It would be nice to have KUBSAN enabled when the daily CI build jobs are running.

yes.

> Would it make sense to create a GENERIC.KUBSAN config for all ports so build issues as shown above are caught?

at least, it doesn't seem an awful idea.

> And what about running the daily test runs with the supported kernel sanitizers enabled as well?

we do have syzkaller doing this (at least, we usually do.)

> It's a shame that such useful functionality is not enabled by default at least when the daily test runs are executed.

have a look at syzkaller pages.  we have a lot to fix :-)



.mrg.