Re: Proposal, again: Disable autoload of compat_xyz modules
In article <20190927125444.gb12...@pony.stderr.spb.ru>,
Valery Ushakov wrote:
>
>May be we should take a look at how SNMP did tables in MIB, b/c we are
>trying to create just such a table indexed by module name.
I think it is simpler than that.
>
>Also, I'm not that sure about autoload of compat stuff especially
>since iirc it currently implies auto-unload too. I vaguely remember
>when I was debugging something in sh3 kobj_machdep.c I had some
>printfs there that made the autoloads visibile and (iirc) each vi
>invocation would trigger an autoload of compat ioctl code (which
>wouldn't recognize the ioctl, and that would be auto-unloaded a few
>seconds later).
>
>-uwe
Here's what I've implemented:
kern.module.noautoload="compat_linux* compat_[0-4]?"
This disables autoload for all compat_linux modules as well
as compat_netbsd < NetBSD-5.0
Comments?
christos
Index: kern_exec.c
===
RCS file: /cvsroot/src/sys/kern/kern_exec.c,v
retrieving revision 1.481
diff -u -u -r1.481 kern_exec.c
--- kern_exec.c 17 Sep 2019 15:19:27 - 1.481
+++ kern_exec.c 28 Sep 2019 01:27:00 -
@@ -626,6 +626,8 @@
"exec_ecoff",
"compat_aoutm68k",
"compat_netbsd32",
+ "compat_linux",
+ "compat_linux32",
"compat_sunos",
"compat_sunos32",
"compat_ultrix",
Index: kern_module.c
===
RCS file: /cvsroot/src/sys/kern/kern_module.c,v
retrieving revision 1.138
diff -u -u -r1.138 kern_module.c
--- kern_module.c 8 Aug 2019 18:08:41 - 1.138
+++ kern_module.c 28 Sep 2019 01:27:00 -
@@ -53,10 +53,16 @@
#include
#include
#include
+#include
#include
#include
+#ifndef MODULE_NOAUTOLOAD
+// Disable compat_linux and compat_linux32 by default for now
+#define MODULE_NOAUTOLOAD "compat_linux*"
+#endif
+
struct vm_map *module_map;
const char *module_machine;
char module_base[MODULE_BASE_SIZE];
@@ -93,6 +99,7 @@
u_int module_gen = 1;
static kcondvar_t module_thread_cv;
static kmutex_t module_thread_lock;
+static kmutex_t module_noautoload_lock;
static int module_thread_ticks;
int (*module_load_vfs_vec)(const char *, int, bool, module_t *,
prop_dictionary_t *) = (void *)eopnotsupp;
@@ -120,6 +127,8 @@
static voidmodule_enqueue(module_t *);
static boolmodule_merge_dicts(prop_dictionary_t, const prop_dictionary_t);
+static boolmodule_allow_autoload(const char *);
+static voidmodule_noautoload_update(const char *);
static voidsysctl_module_setup(void);
static int sysctl_module_autotime(SYSCTLFN_PROTO);
@@ -412,6 +421,7 @@
}
cv_init(_thread_cv, "mod_unld");
mutex_init(_thread_lock, MUTEX_DEFAULT, IPL_NONE);
+ mutex_init(_noautoload_lock, MUTEX_DEFAULT, IPL_NONE);
TAILQ_INIT();
#ifdef MODULAR /* XXX */
@@ -444,6 +454,8 @@
module_netbsd = module_newmodule(MODULE_SOURCE_KERNEL);
module_netbsd->mod_refcnt = 1;
module_netbsd->mod_info = _netbsd_modinfo;
+
+ module_noautoload_update(MODULE_NOAUTOLOAD);
}
/*
@@ -503,6 +515,100 @@
return (0);
}
+struct noautoload {
+ const char *name;
+ SLIST_ENTRY(noautoload) next;
+};
+
+static SLIST_HEAD(, noautoload) noautoload_list =
+SLIST_HEAD_INITIALIZER(noautoload_list);
+
+static char noautoload_buf[1024];
+static char noautoload_nbuf[sizeof(noautoload_buf)];
+
+static void
+module_noautoload_update(const char *cbuf)
+{
+ static const char SEP[] = " \t\n,";
+ struct noautoload *e, *te;
+ char buf[sizeof(noautoload_buf)];
+
+ strlcpy(buf, cbuf, sizeof(buf));
+
+ mutex_enter(_noautoload_lock);
+
+ SLIST_FOREACH_SAFE(e, _list, next, te) {
+ SLIST_REMOVE(_list, e, noautoload, next);
+ kmem_free(e, sizeof(*e));
+ }
+
+ noautoload_nbuf[0] = noautoload_buf[0] = '\0';
+
+ size_t pos = 0;
+ char *p, *str = buf;
+
+ while ((p = strsep(, SEP)) != NULL) {
+ size_t len = strlen(p);
+ if (len == 0)
+ break;
+ e = kmem_alloc(sizeof(*e), KM_SLEEP);
+ e->name = noautoload_nbuf + pos;
+ SLIST_INSERT_HEAD(_list, e, next);
+
+ memcpy(noautoload_buf + pos, p, len);
+ memcpy(noautoload_nbuf + pos, p, len);
+ pos += len;
+ noautoload_buf[pos] = ' ';
+ noautoload_nbuf[pos] = '\0';
+ pos++;
+ }
+
+ if (pos)
+ noautoload_buf[pos - 1] = '\0'; /* space to NUL */
+
+ mutex_exit(_noautoload_lock);
+}
+
+static int
+sysctl_module_noautoload(SYSCTLFN_ARGS)
+{
+ struct sysctlnode node;
+ int error;
+ char newbuf[sizeof(noautoload_buf)];
+
+
Re: panic: UBSan: Undefined Behavior in /syzkaller/managers/netbsd-kubsan/kernel/sys/kern/kern_rndq.c:LINE, negation of -ADD Reply-To:
On Fri 27 Sep 2019 at 21:12:13 +0200, Kamil Rytarowski wrote:
> Thank you for the analysis. Please prepare a patch and commit. Please
> add in the message:
>
> Reported-by: syzbot+68c37d09c833f8ec1...@syzkaller.appspotmail.com
How about this patch. I managed to avoid getting into 64-bit
calculations because I realised that all sample values are uint32_t.
Absolute delta values of those also fit into uint32_t. If you always do
the subtraction in the right order, you never even get a negative value
that you have to correct. Overall I also didn't introduce more
conditional jumps, but I have probably changed the amount of code in
each arm of the conditions.
For review, I left a bunch of //-comments in that I'd remove before
committing but they may be useful to follow the changes. Since the diff
may be a bit annoying to read, I also include the new version of the
code (it is all fairly closely together).
I compile-tested the below but have not run it.
--- kern_rndq.c.orig2019-09-27 21:47:28.066249621 +0200
+++ kern_rndq.c 2019-09-27 22:44:13.307581068 +0200
@@ -324,22 +324,30 @@
* non-zero. If any of these are zero, return zero.
*/
static inline uint32_t
-rnd_delta_estimate(rnd_delta_t *d, uint32_t v, int32_t delta)
+rnd_delta_estimate(rnd_delta_t *d, uint32_t v, uint32_t delta)
{
- int32_t delta2, delta3;
+ uint32_t delta2, delta3;
d->insamples++;
/*
* Calculate the second and third order differentials
*/
- delta2 = d->dx - delta;
- if (delta2 < 0)
- delta2 = -delta2;
-
- delta3 = d->d2x - delta2;
- if (delta3 < 0)
- delta3 = -delta3;
+ if (delta > d->dx)
+ delta2 = delta - d->dx; // u32 - u32 makes u32
+ else
+ delta2 = d->dx - delta; // u32 - u32 makes u32
+ // can't happen anymore:
+// if (delta2 < 0)
+// delta2 = -delta2; /* back to ~32 bits */
+
+ if (delta2 > d->dx)
+ delta3 = delta2 - d->d2x; // u32 - u32 makes u32
+ else
+ delta3 = d->d2x - delta2; // u32 - u32 makes u32
+ // can't happen anymore:
+// if (delta3 < 0)
+// delta3 = -delta3;
d->x = v;
d->dx = delta;
@@ -357,24 +365,29 @@
}
/*
- * Delta estimator for 32-bit timeestamps. Must handle wrap.
+ * Delta estimator for 32-bit timestamps.
+ * Timestaps generally increase, but may wrap around to 0.
+ * If t decreases, it is assumed that wrap-around occurred (once).
*/
static inline uint32_t
rnd_dt_estimate(krndsource_t *rs, uint32_t t)
{
- int32_t delta;
+ uint32_t delta;
uint32_t ret;
rnd_delta_t *d = >time_delta;
- if (t < d->x) {
- delta = UINT32_MAX - d->x + t;
+ if (t < (uint32_t)d->x) {
+ // t wrapped around
+ delta = UINT32_MAX - (uint32_t)d->x + t;
} else {
- delta = d->x - t;
+ // t >= d->x, the normal case of slightly increasing time
+ delta = t - (uint32_t)d->x;
}
- if (delta < 0) {
- delta = -delta;
- }
+// can't happen any more:
+// //if (delta < 0) {
+// delta = -delta;
+// }
ret = rnd_delta_estimate(d, t, delta);
@@ -391,21 +404,27 @@
}
/*
- * Delta estimator for 32 or bit values. "Wrap" isn't.
+ * Delta estimator for arbitrary unsigned 32 bit values.
*/
static inline uint32_t
rnd_dv_estimate(krndsource_t *rs, uint32_t v)
{
- int32_t delta;
+ uint32_t delta;
uint32_t ret;
rnd_delta_t *d = >value_delta;
- delta = d->x - v;
-
- if (delta < 0) {
- delta = -delta;
+ if (v >= (uint32_t)d->x) {
+ // u32 - u32 makes u32
+ delta = v - (uint32_t)d->x;
+ } else {
+ delta = (uint32_t)d->x - v;
}
- ret = rnd_delta_estimate(d, v, (uint32_t)delta);
+
+ // can't happen anymore
+// if (delta < 0) {
+// delta = -delta;
+// }
+ ret = rnd_delta_estimate(d, v, delta);
KASSERT(d->x == v);
KASSERT(d->dx == delta);
-
new code:
/*
* Use the timing/value of the event to estimate the entropy gathered.
* If all the differentials (first, second, and third) are non-zero, return
* non-zero. If any of these are zero, return zero.
*/
static inline uint32_t
rnd_delta_estimate(rnd_delta_t *d, uint32_t v, uint32_t delta)
{
uint32_t delta2, delta3;
d->insamples++;
/*
* Calculate the second and third order differentials
*/
if (delta > d->dx)
delta2 = delta - d->dx; // u32 - u32 makes u32
else
delta2 = d->dx - delta; // u32 - u32 makes u32
// can't happen anymore:
// if (delta2 < 0)
// delta2 = -delta2; /* back to ~32 bits */
if (delta2 > d->dx)
delta3 = delta2 - d->d2x;
Re: panic: UBSan: Undefined Behavior in /syzkaller/managers/netbsd-kubsan/kernel/sys/kern/kern_rndq.c:LINE, negation of -ADD Reply-To:
On 27.09.2019 20:50, Rhialto wrote: > On Fri 27 Sep 2019 at 15:53:47 +0200, Kamil Rytarowski wrote: >> On 27.09.2019 10:19, Rhialto wrote: >>> On Thu 26 Sep 2019 at 01:15:23 +0200, Kamil Rytarowski wrote: Is this patch correct? http://netbsd.org/~kamil/patch-00168-kern_rndq.c-avoid-overflow.txt This code will map the corner case into defined semantics, treating delta INT32_MIN as INT32_MIN+1. >>> >>> I don't know if it is important, but it makes the "normalized" delta >>> value INT32_MAX more likely than it probably should be, since it can >>> result from "raw" delta values INT32_MIN, INT32_MIN+1, and INT32_MAX. >>> >>> Similarly, 0 is underrepresented as output, since it only can result >>> from 0. >>> >>> Other positive results x can come from x and -x as inputs. >>> >>> -Olaf. >>> >> >> If you think that it shall be represented a 0, I can go for it. >> > I have been pondering it for a while and it seems more complicated than > I first thought... it seems that the actual value of delta isn't even > important, but what rnd_delta_estimate() makes of it. That calculates a > derivative and a second derivative. So then one would think that > creating discontinities in the time sequence of delta values, since that > would likely lead to mis-estimations. (Or at least big changes compared > to what it calculates now) > > But if the actual magnitude of delta isn't even important (due to the > differentials), then one could get rid of all the negative delta values > by simply always using delta-INT32_MIN (a.k.a. delta+INT32_MAX+1). > That maps the range [ INT32_MIN, INT32_MAX ] to [0, UINT32_MAX ]. > > To bad that rnd_delta_estimate() doesn't accept unsigned integers but > only signed ones. (Why? A delta between two n-bit integerrs needs n+1 > bits, so one would expect it to take int64s...) > > To correct for that, maybe (delta-INT_MIN)/2 would be a solution (if > calculated without causing overflows). > > The different calculations throw away information in different ways: the > current code that essentially takes abs(delta) sees a no entropy in the > delta sequence 1,-1,1,-1, But the (delta-INT_MIN)/2 method sees > nothing in 1,2,1,2,... > > So my final thought is that it is perhaps best to adjust > rnd_delta_estimate() to take delta in an int64_t or uint32_t. After > all, rnd_delta_t already contains uint64_ts for the first and second > order differentials. With 32-bit samples and 33-bit deltas, 64-bits are > certainly enough there. Then (delta-INT_MIN) can simply be used. > > -Olaf. > Thank you for the analysis. Please prepare a patch and commit. Please add in the message: Reported-by: syzbot+68c37d09c833f8ec1...@syzkaller.appspotmail.com signature.asc Description: OpenPGP digital signature
Re: panic: UBSan: Undefined Behavior in /syzkaller/managers/netbsd-kubsan/kernel/sys/kern/kern_rndq.c:LINE, negation of -ADD Reply-To:
On Fri 27 Sep 2019 at 15:53:47 +0200, Kamil Rytarowski wrote: > On 27.09.2019 10:19, Rhialto wrote: > > On Thu 26 Sep 2019 at 01:15:23 +0200, Kamil Rytarowski wrote: > >> Is this patch correct? > >> > >> http://netbsd.org/~kamil/patch-00168-kern_rndq.c-avoid-overflow.txt > >> > >> This code will map the corner case into defined semantics, treating > >> delta INT32_MIN as INT32_MIN+1. > > > > I don't know if it is important, but it makes the "normalized" delta > > value INT32_MAX more likely than it probably should be, since it can > > result from "raw" delta values INT32_MIN, INT32_MIN+1, and INT32_MAX. > > > > Similarly, 0 is underrepresented as output, since it only can result > > from 0. > > > > Other positive results x can come from x and -x as inputs. > > > > -Olaf. > > > > If you think that it shall be represented a 0, I can go for it. > I have been pondering it for a while and it seems more complicated than I first thought... it seems that the actual value of delta isn't even important, but what rnd_delta_estimate() makes of it. That calculates a derivative and a second derivative. So then one would think that creating discontinities in the time sequence of delta values, since that would likely lead to mis-estimations. (Or at least big changes compared to what it calculates now) But if the actual magnitude of delta isn't even important (due to the differentials), then one could get rid of all the negative delta values by simply always using delta-INT32_MIN (a.k.a. delta+INT32_MAX+1). That maps the range [ INT32_MIN, INT32_MAX ] to [0, UINT32_MAX ]. To bad that rnd_delta_estimate() doesn't accept unsigned integers but only signed ones. (Why? A delta between two n-bit integerrs needs n+1 bits, so one would expect it to take int64s...) To correct for that, maybe (delta-INT_MIN)/2 would be a solution (if calculated without causing overflows). The different calculations throw away information in different ways: the current code that essentially takes abs(delta) sees a no entropy in the delta sequence 1,-1,1,-1, But the (delta-INT_MIN)/2 method sees nothing in 1,2,1,2,... So my final thought is that it is perhaps best to adjust rnd_delta_estimate() to take delta in an int64_t or uint32_t. After all, rnd_delta_t already contains uint64_ts for the first and second order differentials. With 32-bit samples and 33-bit deltas, 64-bits are certainly enough there. Then (delta-INT_MIN) can simply be used. -Olaf. -- Olaf 'Rhialto' Seibert -- rhialto at falu dot nl ___ Anyone who is capable of getting themselves made President should on \X/ no account be allowed to do the job. --Douglas Adams, "THGTTG" signature.asc Description: PGP signature
Re: Proposal, again: Disable autoload of compat_xyz modules
> I guess the main fear is that the attacker can put a malicious (and likely > explicitly crafted for a certain bug in NetBSD's linux compat) binary on > your machine and exectue it. Yes, I guess that's the (valid) point. My impression (I stay corrected) is that compat_linux is mostly used to run a very restricted set of Linux binaries (proprietary software not available for NetBSD) on a NetBSD host. So what would actually be needed (I guess) is a way to restrict emulation (actually running that emulation, not auto-loading the module) to a known set of binaries. I have no idea whether that's possible.
Re: Proposal, again: Disable autoload of compat_xyz modules
> and I'm making a proposal to disable this feature, to prevent this trouble > in the future. Nothing controversial about that, it just makes sense. > > You are not worth my time. Would you mind taking a deep breath and realize that I'm NOT OBJECTING to your proposal? I was just trying to say that, in my opinion, you are doing neither yourself nor your valid proposal a favour by trying to back your proposal with controversal or disputable claims (about compat_linux user base, its usefulness or usability)---claims that are unnecessary (or so I think) for backing your proposal. The point I was objecting to was your claim that compat_linux was of "little actual use" (because, for me, it's vital for keeping my file server running NetBSD).
Re: Proposal, again: Disable autoload of compat_xyz modules
On Fri, Sep 27, 2019 at 12:41:53AM -0400, Mouse wrote: > > Do you want to contribute and do all the actual dirty work for once, > > or you're just here to talk and give your random opinions on things > > you've never invested yourself in? What is your background? What is > > your portfolio? > > You're right that I'm not particularly invested in the original issue; > I don't use Linux compatability myself and I don't run NetBSD more > recent than 5.2 on my own machines (at one of my jobs it looks as > though 8.0 will get used - the reasons I don't run it don't apply > there). I got involved in this thread because invalid "arguments" like > made-up usage statistics bother me. I'm not opposed to the change per > se (I'm not sure whether I think it would actually be a good thing); > I'm opposed to doing it for specious reasons like "<1% users > interested". > > My background? B.Sc. mathematics major, computer science minor. > Programming since the late '70s, been making my living programming > and/or sysadminning since 1984. Been involved with UNIX as a user > since BSD 4.1c, with NetBSD since shortly before the split that led to > OpenBSD. > > My portfolio? Look for "der Mouse" in mail-index.n.o's archives for my > past presence on the lists (including source-changes for what I did > back when I was still committing). Look for > mo...@rodents-montreal.org for my presence after I dropped the "der" - > possibly mo...@rodents.montreal.qc.ca too; I forget which change > occurred first. Look at ftp.rodents-montreal.org:/mouse/git-repos and > clone whichever of them you care to. Perhaps the most notable ones are > moussh, git-interactive, livebackup, Mouse/games/blockade, > Mouse/dreamcast, Mouse/mousemuck, Mouse/vax-emul, Mouse/netbsd-fork/*. > Historically, there's also Mouse-X (an X implementation for 2bpp NeXT > hardware - that one isn't a git repo AFAIK). Look at > ftp.rodents-montreal.org; much of what's there is old versions of stuff > I now keep in git repos (I really kneed to clean it up), but there's > probably at least a little other stuff there. Probably the coolest thing is resize_ffs! Not sure you noticed, but now ARM images will resize themselves to the full size at boot, so to get a machine functional, all you need is to dd an image to an SD card and plug it in. It's so very convenient.
Re: Proposal, again: Disable autoload of compat_xyz modules
On Fri, 27 Sep 2019, Valery Ushakov wrote:
On Fri, Sep 27, 2019 at 11:36:08 -, Christos Zoulas wrote:
} I propose something very slightly different that can preserve the current
} functionality with user action:
}
} 1. Remove them from standard kernels in architectures where modules are
}supported. Users can add them back or just use modules.
} 2. Disable autoloading, but provide a sysctl to enable autoloading
}(1 global sysctl for all compat modules). Users can change the default
}in /etc/sysctl.conf (adds sysctl to the proposal)
You mean this (first line):
i386devel: {31} sysctl kern.module
kern.module.autoload = 0
kern.module.verbose = 0
kern.module.path = /stand/amd64-xen/8.99.26/modules
kern.module.autotime = 10
Perhaps:
kern.module.autoload.disable = linux,linux32
May be we should take a look at how SNMP did tables in MIB, b/c we are
trying to create just such a table indexed by module name.
Also, I'm not that sure about autoload of compat stuff especially
since iirc it currently implies auto-unload too. I vaguely remember
when I was debugging something in sh3 kobj_machdep.c I had some
printfs there that made the autoloads visibile and (iirc) each vi
invocation would trigger an autoload of compat ioctl code (which
wouldn't recognize the ioctl, and that would be auto-unloaded a few
seconds later).
kern.module.autotime controls the delay before the auto-unload takes
place. I think if you set it to zero, it will never auto-unload.
++--+---+
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com |
| Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org |
++--+---+
Re: Proposal, again: Disable autoload of compat_xyz modules
On Fri, Sep 27, 2019 at 11:36:08 -, Christos Zoulas wrote:
> >} I propose something very slightly different that can preserve the current
> >} functionality with user action:
> >}
> >} 1. Remove them from standard kernels in architectures where modules are
> >}supported. Users can add them back or just use modules.
> >} 2. Disable autoloading, but provide a sysctl to enable autoloading
> >}(1 global sysctl for all compat modules). Users can change the default
> >}in /etc/sysctl.conf (adds sysctl to the proposal)
> >
> > You mean this (first line):
> >
> >i386devel: {31} sysctl kern.module
> >kern.module.autoload = 0
> >kern.module.verbose = 0
> >kern.module.path = /stand/amd64-xen/8.99.26/modules
> >kern.module.autotime = 10
>
> Perhaps:
>
> kern.module.autoload.disable = linux,linux32
May be we should take a look at how SNMP did tables in MIB, b/c we are
trying to create just such a table indexed by module name.
Also, I'm not that sure about autoload of compat stuff especially
since iirc it currently implies auto-unload too. I vaguely remember
when I was debugging something in sh3 kobj_machdep.c I had some
printfs there that made the autoloads visibile and (iirc) each vi
invocation would trigger an autoload of compat ioctl code (which
wouldn't recognize the ioctl, and that would be auto-unloaded a few
seconds later).
-uwe
Re: panic: UBSan: Undefined Behavior in /syzkaller/managers/netbsd-kubsan/kernel/sys/kern/kern_rndq.c:LINE, negation of -ADD
On 27.09.2019 10:19, Rhialto wrote: > On Thu 26 Sep 2019 at 01:15:23 +0200, Kamil Rytarowski wrote: >> Is this patch correct? >> >> http://netbsd.org/~kamil/patch-00168-kern_rndq.c-avoid-overflow.txt >> >> This code will map the corner case into defined semantics, treating >> delta INT32_MIN as INT32_MIN+1. > > I don't know if it is important, but it makes the "normalized" delta > value INT32_MAX more likely than it probably should be, since it can > result from "raw" delta values INT32_MIN, INT32_MIN+1, and INT32_MAX. > > Similarly, 0 is underrepresented as output, since it only can result > from 0. > > Other positive results x can come from x and -x as inputs. > > -Olaf. > If you think that it shall be represented a 0, I can go for it. signature.asc Description: OpenPGP digital signature
Re: Proposal, again: Disable autoload of compat_xyz modules
On Fri, Sep 27, 2019 at 10:57:12 +0200, Jarom?r Dole?ek wrote: > Le jeu. 26 sept. 2019 ? 18:08, Manuel Bouyer a ?crit > : > > > > On Thu, Sep 26, 2019 at 05:10:01PM +0200, Maxime Villard wrote: > > > issues for a clearly marginal use case, and given the current general > > ^^^ > > > > This is where we dissagree. You guess it's marginal but there's no > > evidence of that (and there's no evidence of the opposite either). > > FYI - I've put also a lot of efford into fixing & enhancing > compat_linux in past. I also greatly appreciate all the work work of > other folks working on the layer, it's super useful in some situations > - browser with flash support used to be important (thankfully not > anymore), also vmware and matlab, I also used some Oracle dev tools. > However, that is not the topic of the discussion. > > Let's concentrate on whether it should be enabled by default. Yes, please. This discussion has veered way off topic. > Given the history, to me it's completely clear compat_linux shouldn't > be on by default. Any possible linux-specific exploits should only be > problem for people actually explicitly enabling it. Let's just stop > pretending that we'd setup any kind of reasonable testing suite for > this - it has not been done in last >20 years, it's even less likely > to happen now that most of the major use cases are actually moot. > > As Maya suggested, let's keep this concentrated on COMPAT_LINUX only > to avoid further bikeshed flogging, so basically I propose doing this: > 1) Comment out COMPAT_LINUX from all kernels configs for all archs > which support modular > 2) Disable autoload for compat_linux, requiring the user to explicitly > configure system to load it. No extra sysctl. > > Any major and specific objections? At some point it became very hard to follow the technical content of this thread, but I don't think there were any. Thanks! -uwe
Re: Proposal, again: Disable autoload of compat_xyz modules
In article <201909262031.x8qkvnpv021...@server.cornerstoneservice.ca>,
John Nemeth wrote:
>On Sep 26, 7:40pm, Christos Zoulas wrote:
>} In article <390f4c81-bf1c-443f-f7a9-a379c46b7...@m00nbsd.net>,
>} Maxime Villard wrote:
>} >I recently made a big set of changes to fix many bugs and vulnerabilities in
>} >compat_linux and compat_linux32, the majority of which have a security
>impact
>} >bigger than the Intel CPU bugs we hear about so much. These compat layers
>are
>} >enabled by default, so everybody is affected.
>} >
>} >Secteam is in a state where no one is willing to pull up all the changes to
>} >the stable branches, because of the effort. No one is willing to write a
>} >security advisory either. When I say "no one", it includes me.
>} >
>} >The proposal and discussion held in this 2017 thread still hold and are
>} >unchanged two years later:
>} >
>} >https://mail-index.netbsd.org/tech-kern/2017/07/31/msg022153.html
>} >
>} >The compat layers are largely untested, often broken, and are a security
>risk
>} >for everybody. Keeping them enabled for the <1% users interested
>means keeping
>} >vulnerabilities for the >99% who don't use these features.
>} >
>} >In the conversation above, we hit the problem that there was
>cross-dependency
>} >among compat modules, and we couldn't selectively disable specific layers.
>} >Today this is possible thanks to pgoyette's work. That is, it is possible to
>} >comment out "options COMPAT_LINUX" from GENERIC, and have a
>compat_linux.kmod
>} >which will modload correctly and be able to run Linux binaries out of
>the box.
>} >Under this scheme, the feature would be only one root command away from
>being
>} >enabled in the kernel.
>} >
>} >Therefore, I am making today the same proposal as Taylor in 2017, because
>the
>} >problem is still there exactly as-is and we just hit it again; the solution
>} >however is more straightforward.
>}
>} I propose something very slightly different that can preserve the current
>} functionality with user action:
>}
>} 1. Remove them from standard kernels in architectures where modules are
>}supported. Users can add them back or just use modules.
>} 2. Disable autoloading, but provide a sysctl to enable autoloading
>}(1 global sysctl for all compat modules). Users can change the default
>}in /etc/sysctl.conf (adds sysctl to the proposal)
>
> You mean this (first line):
>
>i386devel: {31} sysctl kern.module
>kern.module.autoload = 0
>kern.module.verbose = 0
>kern.module.path = /stand/amd64-xen/8.99.26/modules
>kern.module.autotime = 10
Perhaps:
kern.module.autoload.disable = linux,linux32
christos
Re: Proposal, again: Disable autoload of compat_xyz modules
On Fri, Sep 27, 2019 at 08:30:40AM +0200, Martin Husemann wrote: > On Thu, Sep 26, 2019 at 09:40:22PM +0200, tlaro...@polynum.com wrote: > > If the vulnerabilities can only be exploited by running Linux binaries, > > IMHO, the point is moot: the ones that don't run Linux binaries are not > > affected; the ones that do need to run some Linux binaries will have to > > add the feature so this adds a user's intervention for the very same > > result at the end. > > I guess the main fear is that the attacker can put a malicious (and likely > explicitly crafted for a certain bug in NetBSD's linux compat) binary on > your machine and exectue it. If you have no untrusted local users > and no admin installed linux binaries, the risc should be quite small. Well, I don't think "trusted local users" exist anymore. Because they bring with them (or is it the reverse? The device brings them) i-phones or whatever and connect them, and download applications... Slightly related: is NetBSD providing build services so that someone, not wanting to open his sources, could at least build his program for NetBSD without installing it? Because the best way to avoid the compatibility is to have native NetBSD binaries. -- Thierry Laronde http://www.kergis.com/ http://www.sbfa.fr/ Key fingerprint = 0FF7 E906 FBAF FE95 FD89 250D 52B1 AE95 6006 F40C
Re: Proposal, again: Disable autoload of compat_xyz modules
On Fri, Sep 27, 2019 at 10:57:12AM +0200, Jaromír Dole?ek wrote: > [...] > Given the history, to me it's completely clear compat_linux shouldn't > be on by default. Any possible linux-specific exploits should only be > problem for people actually explicitly enabling it. Let's just stop > pretending that we'd setup any kind of reasonable testing suite for > this - it has not been done in last >20 years, it's even less likely > to happen now that most of the major use cases are actually moot. > > As Maya suggested, let's keep this concentrated on COMPAT_LINUX only > to avoid further bikeshed flogging, so basically I propose doing this: > 1) Comment out COMPAT_LINUX from all kernels configs for all archs > which support modular > 2) Disable autoload for compat_linux, requiring the user to explicitly > configure system to load it. No extra sysctl. > > Any major and specific objections? not from me. -- Manuel Bouyer NetBSD: 26 ans d'experience feront toujours la difference --
Re: Proposal, again: Disable autoload of compat_xyz modules
Le jeu. 26 sept. 2019 à 18:08, Manuel Bouyer a écrit : > > On Thu, Sep 26, 2019 at 05:10:01PM +0200, Maxime Villard wrote: > > issues for a clearly marginal use case, and given the current general > ^^^ > > This is where we dissagree. You guess it's marginal but there's no > evidence of that (and there's no evidence of the opposite either). FYI - I've put also a lot of efford into fixing & enhancing compat_linux in past. I also greatly appreciate all the work work of other folks working on the layer, it's super useful in some situations - browser with flash support used to be important (thankfully not anymore), also vmware and matlab, I also used some Oracle dev tools. However, that is not the topic of the discussion. Let's concentrate on whether it should be enabled by default. Given the history, to me it's completely clear compat_linux shouldn't be on by default. Any possible linux-specific exploits should only be problem for people actually explicitly enabling it. Let's just stop pretending that we'd setup any kind of reasonable testing suite for this - it has not been done in last >20 years, it's even less likely to happen now that most of the major use cases are actually moot. As Maya suggested, let's keep this concentrated on COMPAT_LINUX only to avoid further bikeshed flogging, so basically I propose doing this: 1) Comment out COMPAT_LINUX from all kernels configs for all archs which support modular 2) Disable autoload for compat_linux, requiring the user to explicitly configure system to load it. No extra sysctl. Any major and specific objections? Jaromir
Re: panic: UBSan: Undefined Behavior in /syzkaller/managers/netbsd-kubsan/kernel/sys/kern/kern_rndq.c:LINE, negation of -ADD
On Thu 26 Sep 2019 at 01:15:23 +0200, Kamil Rytarowski wrote: > Is this patch correct? > > http://netbsd.org/~kamil/patch-00168-kern_rndq.c-avoid-overflow.txt > > This code will map the corner case into defined semantics, treating > delta INT32_MIN as INT32_MIN+1. I don't know if it is important, but it makes the "normalized" delta value INT32_MAX more likely than it probably should be, since it can result from "raw" delta values INT32_MIN, INT32_MIN+1, and INT32_MAX. Similarly, 0 is underrepresented as output, since it only can result from 0. Other positive results x can come from x and -x as inputs. -Olaf. -- Olaf 'Rhialto' Seibert -- rhialto at falu dot nl ___ Anyone who is capable of getting themselves made President should on \X/ no account be allowed to do the job. --Douglas Adams, "THGTTG" signature.asc Description: PGP signature
[no subject]
from Kamil Rytarowski:
> I have managed to get light cross-toolchain producing Linux binaries.
> The only dependency is musl (+gmake to build it).
Is it necessary to use musl, as opposed to uClibc-ng or glibc?
Did you cross-compile from NetBSD to get the cross-toolchain, or did you need
some Linux stuff already compiled?
But I believe you would need Linux kernel headers in any case.
Where do you get musl-gcc, or is it built as part of the light cross-toolchain?
> It works almost out of the box (I had to manually build certain files).
> The only think that is required to be tuned is to add a dummy gnu_debuglink.
> https://nxr.netbsd.org/xref/src/sys/compat/linux/common/linux_exec_elf32.c#214
> The kernel shall have a fallback, and probably parse .intrp string and
> look for 'ld-musl' in it.
> $ cat m.c
> #include
> int
> main(int argc, char **argv)
{
> printf("Hello world!\n");
> return 0;
}
> $ musl-gcc m.c
> $ objcopy --add-gnu-debuglink=/dev/null ./a.out
> $ file a.out
> a.out: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically
> linked, interpreter /lib/ld-musl-x86_64.so.1, not stripped
> $ ./a.out
> Hello world!
> Another step is to build LTP with musl-gcc and run the tests.
> https://github.com/linux-test-project/ltp (LTP supports musl)
> There will be some patches involved.
> The question is how to setup the tests for it? anita + picking these
> packages from pkgsrc?
> Making a setup with a syzbot instance for linux32/linux64 target should
> be not that difficult.
> Unfortunately all that needs some work in various projects, integration,
> upstreaming code (to LTP?) etc and maybe TNF could spare someone to do it.
> > Also linux_compat is getting more and more irrelevant as time pass due
> > to shortage in our futex code (lack of robust futexes).
> This is slowly worked on.
Tom
Re: Proposal, again: Disable autoload of compat_xyz modules
On Thu, Sep 26, 2019 at 09:40:22PM +0200, tlaro...@polynum.com wrote: > If the vulnerabilities can only be exploited by running Linux binaries, > IMHO, the point is moot: the ones that don't run Linux binaries are not > affected; the ones that do need to run some Linux binaries will have to > add the feature so this adds a user's intervention for the very same > result at the end. I guess the main fear is that the attacker can put a malicious (and likely explicitly crafted for a certain bug in NetBSD's linux compat) binary on your machine and exectue it. If you have no untrusted local users and no admin installed linux binaries, the risc should be quite small. If you have local users, the risc is pretty high. The relatively bad testing situation for this compat layers could easily lead to non-malicious linux binaries running into strange error cases and triggereing corner cases in the compat layers (most likely variant: a user tries some random linux binary, the emulation is incomplete and the kernel crashes). There are other vectors to get to this vulnerabilities, but they all require exploiting some other serious bug first. Martin
