re: NetBSD Kernel Sanitizers

matthew green <[email protected]>
Newsgroups gmane.os.netbsd.devel.kernel
Message-ID <[email protected]>
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.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.