Re: dt(4) and allowkmem
Sure.
There may be some man page locations missing, from a grep:
man2/sysctl.2:.It Dv KERN_ALLOWKMEM Pq Va kern.allowkmem
man3/sysctl.3:.It Dv KERN_ALLOWKMEM Pq Va kern.allowkmem
man7/securelevel.7:.Va kern.allowkmem ,
Martin Pieuchot wrote:
> On 22/01/20(Wed) 14:56, Theo de Raadt wrote:
> > Todd C. Miller wrote:
> >
> > > On Wed, 22 Jan 2020 15:12:25 +0100, Martin Pieuchot wrote:
> > >
> > > > dt(4) is a debugging interface that allows userland to read kernel
> > > > addresses. So its access should be restricted by default, just like
> > > > mem(4).
> > > >
> > > > Diff prevent opening the pseudo-device unless `allowkmem' is set.
> > >
> > > Does it really make sense to reuse `allowkmem' for this? This will
> > > mean that in order to use dt(4) you also have to open up mem(4).
> > > I don't think that is desirable.
> >
> > The things you can learn via dt are a stong inspection window into
> > kmem. I think it's stronger than immediately obvious.
> >
> > > If you want to disable dt(4) by default I think you are better off
> > > using a new sysctl knob.
> >
> > I'm on the fence about it. But it is small, so I think allowdt is
> > better.
>
> Sure! Diff below does that, ok?
>
> Index: dev/dt/dt_dev.c
> ===
> RCS file: /cvs/src/sys/dev/dt/dt_dev.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 dt_dev.c
> --- dev/dt/dt_dev.c 21 Jan 2020 16:16:23 - 1.1
> +++ dev/dt/dt_dev.c 23 Jan 2020 08:56:00 -
> @@ -132,6 +132,10 @@ dtopen(dev_t dev, int flags, int mode, s
> {
> struct dt_softc *sc;
> int unit = minor(dev);
> + extern int allowdt;
> +
> + if (!allowdt)
> + return EPERM;
>
> KASSERT(dtlookup(unit) == NULL);
>
> Index: kern/kern_sysctl.c
> ===
> RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
> retrieving revision 1.369
> diff -u -p -r1.369 kern_sysctl.c
> --- kern/kern_sysctl.c2 Jan 2020 08:52:53 - 1.369
> +++ kern/kern_sysctl.c23 Jan 2020 08:54:12 -
> @@ -129,6 +129,7 @@ extern int audio_record_enable;
> #endif
>
> int allowkmem;
> +int allowdt;
>
> int sysctl_diskinit(int, struct proc *);
> int sysctl_proc_args(int *, u_int, void *, size_t *, struct proc *);
> @@ -358,12 +359,14 @@ kern_sysctl(int *name, u_int namelen, vo
> return (EPERM);
> securelevel = level;
> return (0);
> + case KERN_ALLOWDT:
> + if (securelevel > 0)
> + return (sysctl_rdint(oldp, oldlenp, newp, allowdt));
> + return (sysctl_int(oldp, oldlenp, newp, newlen, ));
> case KERN_ALLOWKMEM:
> if (securelevel > 0)
> - return (sysctl_rdint(oldp, oldlenp, newp,
> - allowkmem));
> - return (sysctl_int(oldp, oldlenp, newp, newlen,
> - ));
> + return (sysctl_rdint(oldp, oldlenp, newp, allowkmem));
> + return (sysctl_int(oldp, oldlenp, newp, newlen, ));
> case KERN_HOSTNAME:
> error = sysctl_tstring(oldp, oldlenp, newp, newlen,
> hostname, sizeof(hostname));
> Index: sys/sysctl.h
> ===
> RCS file: /cvs/src/sys/sys/sysctl.h,v
> retrieving revision 1.199
> diff -u -p -r1.199 sysctl.h
> --- sys/sysctl.h 24 Dec 2019 13:13:54 - 1.199
> +++ sys/sysctl.h 23 Jan 2020 08:55:26 -
> @@ -165,7 +165,7 @@ struct ctlname {
> #define KERN_SHMINFO62 /* struct: SysV struct shminfo
> */
> #define KERN_INTRCNT 63 /* node: interrupt counters */
> #define KERN_WATCHDOG 64 /* node: watchdog */
> -/* was KERN_EMUL 65 */
> +#define KERN_ALLOWDT 65 /* int: allowdt */
> #define KERN_PROC 66 /* struct: process entries */
> #define KERN_MAXCLUSTERS67 /* number of mclusters */
> #define KERN_EVCOUNT 68 /* node: event counters */
> @@ -257,7 +257,7 @@ struct ctlname {
> { "shminfo", CTLTYPE_STRUCT }, \
> { "intrcnt", CTLTYPE_NODE }, \
> { "watchdog", CTLTYPE_NODE }, \
> - { "gap", 0 }, \
> + { "allowdt", CTLTYPE_INT }, \
> { "proc", CTLTYPE_STRUCT }, \
> { "maxclusters", CTLTYPE_INT }, \
> { "evcount", CTLTYPE_NODE }, \
Re: dt(4) and allowkmem
On Thu, 23 Jan 2020 10:03:08 +0100, Martin Pieuchot wrote: > Sure! Diff below does that, ok? Looks good. OK millert@ - todd
Re: dt(4) and allowkmem
On 22/01/20(Wed) 14:56, Theo de Raadt wrote:
> Todd C. Miller wrote:
>
> > On Wed, 22 Jan 2020 15:12:25 +0100, Martin Pieuchot wrote:
> >
> > > dt(4) is a debugging interface that allows userland to read kernel
> > > addresses. So its access should be restricted by default, just like
> > > mem(4).
> > >
> > > Diff prevent opening the pseudo-device unless `allowkmem' is set.
> >
> > Does it really make sense to reuse `allowkmem' for this? This will
> > mean that in order to use dt(4) you also have to open up mem(4).
> > I don't think that is desirable.
>
> The things you can learn via dt are a stong inspection window into
> kmem. I think it's stronger than immediately obvious.
>
> > If you want to disable dt(4) by default I think you are better off
> > using a new sysctl knob.
>
> I'm on the fence about it. But it is small, so I think allowdt is
> better.
Sure! Diff below does that, ok?
Index: dev/dt/dt_dev.c
===
RCS file: /cvs/src/sys/dev/dt/dt_dev.c,v
retrieving revision 1.1
diff -u -p -r1.1 dt_dev.c
--- dev/dt/dt_dev.c 21 Jan 2020 16:16:23 - 1.1
+++ dev/dt/dt_dev.c 23 Jan 2020 08:56:00 -
@@ -132,6 +132,10 @@ dtopen(dev_t dev, int flags, int mode, s
{
struct dt_softc *sc;
int unit = minor(dev);
+ extern int allowdt;
+
+ if (!allowdt)
+ return EPERM;
KASSERT(dtlookup(unit) == NULL);
Index: kern/kern_sysctl.c
===
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.369
diff -u -p -r1.369 kern_sysctl.c
--- kern/kern_sysctl.c 2 Jan 2020 08:52:53 - 1.369
+++ kern/kern_sysctl.c 23 Jan 2020 08:54:12 -
@@ -129,6 +129,7 @@ extern int audio_record_enable;
#endif
int allowkmem;
+int allowdt;
int sysctl_diskinit(int, struct proc *);
int sysctl_proc_args(int *, u_int, void *, size_t *, struct proc *);
@@ -358,12 +359,14 @@ kern_sysctl(int *name, u_int namelen, vo
return (EPERM);
securelevel = level;
return (0);
+ case KERN_ALLOWDT:
+ if (securelevel > 0)
+ return (sysctl_rdint(oldp, oldlenp, newp, allowdt));
+ return (sysctl_int(oldp, oldlenp, newp, newlen, ));
case KERN_ALLOWKMEM:
if (securelevel > 0)
- return (sysctl_rdint(oldp, oldlenp, newp,
- allowkmem));
- return (sysctl_int(oldp, oldlenp, newp, newlen,
- ));
+ return (sysctl_rdint(oldp, oldlenp, newp, allowkmem));
+ return (sysctl_int(oldp, oldlenp, newp, newlen, ));
case KERN_HOSTNAME:
error = sysctl_tstring(oldp, oldlenp, newp, newlen,
hostname, sizeof(hostname));
Index: sys/sysctl.h
===
RCS file: /cvs/src/sys/sys/sysctl.h,v
retrieving revision 1.199
diff -u -p -r1.199 sysctl.h
--- sys/sysctl.h24 Dec 2019 13:13:54 - 1.199
+++ sys/sysctl.h23 Jan 2020 08:55:26 -
@@ -165,7 +165,7 @@ struct ctlname {
#defineKERN_SHMINFO62 /* struct: SysV struct shminfo
*/
#define KERN_INTRCNT 63 /* node: interrupt counters */
#defineKERN_WATCHDOG 64 /* node: watchdog */
-/* was KERN_EMUL 65 */
+#define KERN_ALLOWDT 65 /* int: allowdt */
#defineKERN_PROC 66 /* struct: process entries */
#defineKERN_MAXCLUSTERS67 /* number of mclusters */
#define KERN_EVCOUNT 68 /* node: event counters */
@@ -257,7 +257,7 @@ struct ctlname {
{ "shminfo", CTLTYPE_STRUCT }, \
{ "intrcnt", CTLTYPE_NODE }, \
{ "watchdog", CTLTYPE_NODE }, \
- { "gap", 0 }, \
+ { "allowdt", CTLTYPE_INT }, \
{ "proc", CTLTYPE_STRUCT }, \
{ "maxclusters", CTLTYPE_INT }, \
{ "evcount", CTLTYPE_NODE }, \
Re: dt(4) and allowkmem
Todd C. Miller wrote: > On Wed, 22 Jan 2020 15:12:25 +0100, Martin Pieuchot wrote: > > > dt(4) is a debugging interface that allows userland to read kernel > > addresses. So its access should be restricted by default, just like > > mem(4). > > > > Diff prevent opening the pseudo-device unless `allowkmem' is set. > > Does it really make sense to reuse `allowkmem' for this? This will > mean that in order to use dt(4) you also have to open up mem(4). > I don't think that is desirable. The things you can learn via dt are a stong inspection window into kmem. I think it's stronger than immediately obvious. > If you want to disable dt(4) by default I think you are better off > using a new sysctl knob. I'm on the fence about it. But it is small, so I think allowdt is better.
Re: dt(4) and allowkmem
On Wed, 22 Jan 2020 15:12:25 +0100, Martin Pieuchot wrote: > dt(4) is a debugging interface that allows userland to read kernel > addresses. So its access should be restricted by default, just like > mem(4). > > Diff prevent opening the pseudo-device unless `allowkmem' is set. Does it really make sense to reuse `allowkmem' for this? This will mean that in order to use dt(4) you also have to open up mem(4). I don't think that is desirable. If you want to disable dt(4) by default I think you are better off using a new sysctl knob. - todd
dt(4) and allowkmem
dt(4) is a debugging interface that allows userland to read kernel
addresses. So its access should be restricted by default, just like
mem(4).
Diff prevent opening the pseudo-device unless `allowkmem' is set.
ok?
Index: sys/dev/dt/dt_dev.c
===
RCS file: /cvs/src/sys/dev/dt/dt_dev.c,v
retrieving revision 1.1
diff -u -p -r1.1 dt_dev.c
--- sys/dev/dt/dt_dev.c 21 Jan 2020 16:16:23 - 1.1
+++ sys/dev/dt/dt_dev.c 22 Jan 2020 13:59:01 -
@@ -132,6 +132,10 @@ dtopen(dev_t dev, int flags, int mode, s
{
struct dt_softc *sc;
int unit = minor(dev);
+ extern int allowkmem;
+
+ if (!allowkmem)
+ return EPERM;
KASSERT(dtlookup(unit) == NULL);
Index: share/man/man4/dt.4
===
RCS file: /cvs/src/share/man/man4/dt.4,v
retrieving revision 1.1
diff -u -p -r1.1 dt.4
--- share/man/man4/dt.4 21 Jan 2020 16:18:28 - 1.1
+++ share/man/man4/dt.4 22 Jan 2020 14:01:13 -
@@ -28,6 +28,11 @@ It has to be configured and enabled thro
.Xr ioctl 2
interface exposed by the pseudo-device
.Pa /dev/dt .
+.Pp
+This device can only be opened when the
+.Va kern.allowkmem
+.Xr sysctl 2
+variable is set.
.\"Sh IOCTL INTERFACE
.\"
.Sh FILES
