Le 01/01/2018 à 02:17, John Nemeth a écrit :
On Dec 31,  5:11pm, Maxime Villard wrote:
}
} Here is a patch [1] that hides the addresses of the kernel modules when
} 'modstat -k' is entered by an unprivileged user. The current behavior is
} preserved for root.
}
} The addresses currently leaked cannot be used to reconstruct the layout of
} the kernel, since the module VAs are embedded in bootspace.boot, whose 
location
} is independent from that of each of the remaining kernel segments.
}
} But it's still good not to leak such information, to limit the surface for ROP
} and a few other things, and this, also in the non-kaslr case. Ok?
}
} [1] http://m00nbsd.net/garbage/module/modstat.diff

@@ -150,10 +159,13 @@
                        strlcpy(ms->ms_required, mi->mi_required,
                            sizeof(ms->ms_required));
                }
-               if (mod->mod_kobj != NULL) {
+               if (mod->mod_kobj != NULL && stataddr) {
                        kobj_stat(mod->mod_kobj, &addr, &size);
                        ms->ms_addr = addr;
                        ms->ms_size = size;
+               } else {
+                       ms->ms_addr = 0;
+                       ms->ms_size = 0;
                }
                ms->ms_class = mi->mi_class;
                ms->ms_refcnt = -1;

      I don't see why you added the part where you set ms_addr and
ms_size to 0 given that the memory was kmem_zalloc'ed and thus we
know that it is already 0?

Yes it's already zero, but I still added a branch to explicitly show that it
is zero. But that's indeed useless, and I've now removed it.

      Also, given the reason for preventing information leaks, I
would also make sure that the address isn't given out even for root
when secure_level has been elevated.

Actually, that's already the case. See secmodel_securelevel.c, there's

299     case KAUTH_SYSTEM_MODULE:
300             if (securelevel > 0)
301                     result = KAUTH_RESULT_DENY;
302             break;

Maxime

Reply via email to