On Fri, Jan 07, 2022 at 12:25:25PM -0700, Theo de Raadt wrote:
> > I agree, but the intent is replacing a debugging method with another
> > debugging method (hoping it is more useful). The messages showed here
> > are the same that the ones which would be shown on the console before
> > the diff.
>
> the thing about the kernel messages, is you see the pointers and you
> know to go fix the code.
>
> The problem with hiding it in some optional-visibility interface, is
> that these things may show up without the more paranoid developers
> noticing the problem.
>
> How do we prevent non-#define DEBUG_FOO situations from growing?
As the kernel currently contains (at least) 1946 pointers leak by using
printf(9) and %p, it might be more efficient to scramble %p with a
static value (randomly assigned at boot time).
Using xor with such static-random-value will conserve uniqueness and
reproductibility of the printf(9) for the kernel live-time.
The following diff does it.
If the real value is still wanted (but it should be for unmentionable
reasons, as even in debug code is seems to be unwanted</sarcasm>), a
%P format options could be introduced for such case.
Alternatively, %P might be used for print the scrambled value, but it
will imply reviewing 1946 usage of %p.
--
Sebastien Marie
diff 9fbb8b8358a4a6c4112136a2bc60d2cee1a7a243 /home/semarie/repos/openbsd/src
blob - a7de554db1973d60c53eae927b51febc6f547b8e
file + share/man/man9/printf.9
--- share/man/man9/printf.9
+++ share/man/man9/printf.9
@@ -214,6 +214,9 @@ encountered
distinguishable by its value being \*(Le 32 or \*(Ge 128
.Pc ,
or the end of the decoding directive string itself.
+.It Li %p
+The value showed is anonymized to prevent pointer information leaking.
+Uniqueness and reproducibility are preserved for the kernel live-time.
.El
.Sh RETURN VALUES
The
blob - 3ca2a9226b57519b65852db170309d169ee9e751
file + sys/kern/init_main.c
--- sys/kern/init_main.c
+++ sys/kern/init_main.c
@@ -173,6 +173,7 @@ main(void *framep)
struct pdevinit *pdev;
extern struct pdevinit pdevinit[];
extern void disk_init(void);
+ extern u_long prf_rand;
/*
* Initialize the current process pointer (curproc) before
@@ -213,6 +214,9 @@ main(void *framep)
random_start(boothowto & RB_GOODRANDOM); /* Start the flow */
+ /* Initialize prf_rand with a random value */
+ arc4random_buf(&prf_rand, sizeof(prf_rand));
+
/*
* Initialize mbuf's. Do this now because we might attempt to
* allocate mbufs or mbuf clusters during autoconfiguration.
blob - e2ad6cd97b37ad844cddca0af596e862d0ef8060
file + sys/kern/subr_prf.c
--- sys/kern/subr_prf.c
+++ sys/kern/subr_prf.c
@@ -99,6 +99,8 @@ struct mutex kprintf_mutex =
extern int log_open; /* subr_log: is /dev/klog open? */
const char *panicstr; /* arg to first call to panic (used as a flag
to indicate that panic has already been called). */
+u_long prf_rand; /* printf(9) %p pointer anonymizer */
+
#ifdef DDB
/*
* Enter ddb on panic.
@@ -903,7 +905,7 @@ reswitch: switch (ch) {
* defined manner.''
* -- ANSI X3J11
*/
- _uquad = (u_long)va_arg(ap, void *);
+ _uquad = (u_long)va_arg(ap, void *) ^ prf_rand;
base = HEX;
xdigs = "0123456789abcdef";
flags |= HEXPREFIX;
