Re: CVS commit: src/sys/kern

[Steffen Nurpmeso <sdaoden%yandex.com@localhost>]

"Maxime Villard" <maxv%netbsd.org@localhost> wrote:
 |Module Name:  src
 |Committed By: maxv
 |Date:         Tue Jun 24 07:28:23 UTC 2014
 |
 |Modified Files:
 | src/sys/kern: subr_kmem.c
 |
 |Log Message:
 |KMEM_REDZONE+KMEM_POISON is supposed to detect buffer overflows. But it only
 |poisons memory after kmem_roundup_size(), which means that if an overflow
 |occurs in the page padding, it won't be detected.
 |
 |Fix this by making KMEM_REDZONE independent from KMEM_POISON and making it
 |put a 2-byte pattern at the end of each requested buffer, and check it when
 |freeing memory to ensure the caller hasn't written outside \
 |the requested area.

Interesting.
Having no idea of kernel programming i blindly assume that those
pages are somehow isolated against "preceeding pages", so that no
checks of the lower bound are necessary or even useful, and of
course checking wether the address as such can be safely accessed
is also not necessary / done differently.

But, whereas i really think it is a smart idea to use
a mathematically verifieable pattern, how can you be sure that the
pattern doesn't generate values which are extremely common,
especially at E-O-B, such as '\0'?  Shouldn't at least 0 be
replaced with a different value?

(I always used fixed patterns for that, e.g., for S-nail i have
a mix of regular and irregular bit patterns:

   if (__xl.p_ui8p[0] != 0xDE) __i |= 1<<0;\
   if (__xl.p_ui8p[1] != 0xAA) __i |= 1<<1;\
   if (__xl.p_ui8p[2] != 0x55) __i |= 1<<2;\
   if (__xl.p_ui8p[3] != 0xAD) __i |= 1<<3;\
   if (__xl.p_ui8p[4] != 0xBE) __i |= 1<<4;\
   if (__xl.p_ui8p[5] != 0x55) __i |= 1<<5;\
   if (__xl.p_ui8p[6] != 0xAA) __i |= 1<<6;\
   if (__xl.p_ui8p[7] != 0xEF) __i |= 1<<7;\
   if (__i != 0) {\
      (BAD) = TRU1;\
      alert("%p: corrupt lower canary: 0x%02X: %s, line %u",\
         __xl.p_p, __i, mdbg_file, mdbg_line);\
   }\

though i guess havin 0xAA first is even better here.)

--steffen