kill LK_CANRECURSE
Despite being redundant, the LK_CANRECURSE flag isn't checked anywhere.
Ok?
natano
Index: sys/kern/kern_lock.c
===
RCS file: /cvs/src/sys/kern/kern_lock.c,v
retrieving revision 1.46
diff -u -p -r1.46 kern_lock.c
--- sys/kern/kern_lock.c14 Sep 2014 14:17:25 - 1.46
+++ sys/kern/kern_lock.c12 May 2016 08:34:52 -
@@ -80,8 +80,6 @@ lockmgr(struct lock *lkp, u_int flags, v
KASSERT(!((flags & (LK_SHARED|LK_EXCLUSIVE)) ==
(LK_SHARED|LK_EXCLUSIVE)));
- KASSERT(!((flags & (LK_CANRECURSE|LK_RECURSEFAIL)) ==
- (LK_CANRECURSE|LK_RECURSEFAIL)));
KASSERT((flags & LK_RELEASE) ||
(flags & (LK_SHARED|LK_EXCLUSIVE|LK_DRAIN)));
Index: sys/kern/vfs_vnops.c
===
RCS file: /cvs/src/sys/kern/vfs_vnops.c,v
retrieving revision 1.84
diff -u -p -r1.84 vfs_vnops.c
--- sys/kern/vfs_vnops.c19 Mar 2016 12:04:15 - 1.84
+++ sys/kern/vfs_vnops.c12 May 2016 08:34:52 -
@@ -513,9 +513,6 @@ vn_lock(struct vnode *vp, int flags, str
{
int error;
- if ((flags & LK_RECURSEFAIL) == 0)
- flags |= LK_CANRECURSE;
-
do {
if (vp->v_flag & VXLOCK) {
vp->v_flag |= VXWANT;
Index: sys/sys/lock.h
===
RCS file: /cvs/src/sys/sys/lock.h,v
retrieving revision 1.26
diff -u -p -r1.26 lock.h
--- sys/sys/lock.h 23 Sep 2015 15:37:26 - 1.26
+++ sys/sys/lock.h 12 May 2016 08:34:52 -
@@ -50,7 +50,6 @@ struct lock {
#define LK_DRAIN 0x04/* wait for all lock activity to end */
#define LK_RELEASE 0x08/* release any type of lock */
#define LK_NOWAIT 0x10/* do not sleep to await lock */
-#define LK_CANRECURSE 0x20/* allow recursive exclusive lock */
#define LK_RECURSEFAIL 0x40/* fail if recursive exclusive lock */
#define LK_RETRY 0x80/* vn_lock: retry until locked */
Index: share/man/man9/lock.9
===
RCS file: /cvs/src/share/man/man9/lock.9,v
retrieving revision 1.23
diff -u -p -r1.23 lock.9
--- share/man/man9/lock.9 11 Jan 2015 19:34:52 - 1.23
+++ share/man/man9/lock.9 12 May 2016 08:34:52 -
@@ -83,11 +83,9 @@ Flags to specify the lock behaviour perm
the lock.
Valid lock flags are:
.Pp
-.Bl -tag -width "LK_CANRECURSEXX" -compact
+.Bl -tag -width "LK_NOWAITXX" -compact
.It LK_NOWAIT
Processes should not sleep when attempting to acquire the lock.
-.It LK_CANRECURSE
-Processes can acquire the lock recursively.
.El
.El
.It Fn lockmgr "lock" "flags" "slock"
@@ -110,9 +108,8 @@ Stop further shared-access locks, when t
pending upgrade if it exists, then grant an exclusive-access lock.
Only one exclusive-access lock may exist at a time, except that a
process holding an exclusive-access lock may get additional
-exclusive-access locks if it explicitly sets the LK_CANRECURSE flag in
-the lock request, or if the LK_CANRECURSE flag was set when the lock
-was initialised.
+exclusive-access locks unless the LK_RECURSEFAIL flag was set in the lock
+request.
.It LK_RELEASE
Release one instance of a lock.
.It LK_DRAIN
Re: sed/regcomp bug?
Hi Martijn, Martijn van Duren wrote on Tue, May 10, 2016 at 10:34:54PM +0200: > On 05/10/16 20:58, Ingo Schwarze wrote: >> Martijn van Duren wrote on Tue, May 10, 2016 at 08:08:34PM +0200: >>> On 05/10/16 19:29, Ingo Schwarze wrote: Martijn van Duren wrote on Tue, May 10, 2016 at 02:43:54PM +0200: > Index: ./lib/libc/regex/engine.c > === > RCS file: /cvs/src/lib/libc/regex/engine.c,v > retrieving revision 1.19 > diff -u -p -r1.19 engine.c > --- ./lib/libc/regex/engine.c 28 Dec 2015 23:01:22 - 1.19 > +++ ./lib/libc/regex/engine.c 2 May 2016 08:50:20 - > @@ -674,7 +674,7 @@ fast(struct match *m, char *start, char > states fresh = m->fresh; > states tmp = m->tmp; > char *p = start; > - int c = (start == m->beginp) ? OUT : *(start-1); > + int c = (start == m->offp) ? OUT : *(start-1); > int lastc; /* previous c */ > int flagch; > int i; > @@ -758,7 +758,7 @@ slow(struct match *m, char *start, char > states empty = m->empty; > states tmp = m->tmp; > char *p = start; > - int c = (start == m->beginp) ? OUT : *(start-1); > + int c = (start == m->offp) ? OUT : *(start-1); > int lastc; /* previous c */ > int flagch; > int i; i hate to say that this change appears to cause a regression. The regexec(3) manual explicitly says: REG_STARTEND The string is considered to start at [...] Note that a non-zero rm_so does not imply REG_NOTBOL; REG_STARTEND affects only the location of the string, not how it is matched. >>> Right now, the library actually implements that. The test program appended below produces the following output, as documented: rt: regcomp: OK rt: mismatch: regexec() failed to match rt: BOL match: OK rt: ST match: OK With your change, the library now fails to match: rt: regcomp: OK rt: mismatch: regexec() failed to match rt: BOL match: OK rt: ST match: regexec() failed to match I don't think that change is intentional, or is it? >>> This change is intentional. You try to match y on the start of the >>> string. This falsely succeeds in the current library, but is fixed >>> in my change. >> Since this is an intentional change of the way the library works, >> i think the following is needed: >> >> 1. It ought to be separate from fixing sed(1). >> Mixing interface changes of libraries with bug fixes >> in programs using them seems like a bad idea to me. >> >> 2. We need a rationale why the change is useful in general, >> not just for sed(1), and why we think that nobody relies on the >> current interface definition, or how programs that rely on the >> current behaviour can be found and adjusted to work with the >> new behaviour. >> >> 3. There should be a list explaining how other implementations >> behave in this respect, in particular FreeBSD, NetBSD, >> DragonFly, glibc, illumos or Solaris, and maybe some other >> commercial Unixes. >> >> 4. The library must be kept consistent. For example, you only >> change the treatment of "^" and "\<", but leave "$" and "\>" >> unchanged, see the test program below. That is unlikely to >> be the only inconsistency you introduce. In the function >> backref(), there are multiple references to beginp in the >> immediate vicinity of REG_NOTBOL and ISWORD. I did not >> check the details, but i fear that your change introduces >> a second inconsistency between the treatment of anchors >> inside and outside of backreferences. These are just two >> examples of (potential) inconsistencies, we have to convince >> ourselves there aren't more. >> >> 5. The change needs to be documented. It blantantly contradicts >> what the manual currently says. The regular expression >> manuals are among the most precise we have, and we should >> really keep that level of perfection. >> >> I'm not saying we don't want such a change, nor am i saying we want >> it, but as it stands, the patch is inconsistent, incomplete, and >> mixed up with unrelated stuff, so it is not OK. > I reread the REG_STARTEND section again and it turns out I > misinterpreted it's intention and I thought the current behaviour > was wrong/incomplete. So the change was intentional, but for the > wrong reasons. > > I changed the diff to allow regexec to combine REG_NOTBOL and > REG_STARTEND. This way the current syntax stays intact and we can > use the extra flag to do lookbacks to verify that the last > character was a word character or not, without risking going into > unallocated memory. > > Does this look better to you? Probably minimally better, but still not OK. All the above reservations still apply: 1. It ch
Re: pool related crashes, but "kernel did no panic"
On Wed, Apr 27, 2016 at 7:22 PM, Theo de Raadt wrote: >> On 27/04/16(Wed) 15:45, Alexey Suslikov wrote: >> > Theo de Raadt cvs.openbsd.org> writes: >> > >> > > >> > > Most of these bug reports completely stink. >> > > >> > > ALWAYS include *ALL* information in a report. >> > >> > In an idealistic world, yes. >> >> In an idealistic world their would be no bug. > > In an idealistic world, Alexey Suslikov wouldn't feel compelled to > defend sloppiness. follow up is here http://marc.info/?l=openbsd-bugs&m=146304833425471&w=2 http://marc.info/?l=openbsd-bugs&m=146304864925575&w=2
m_dup_pkt() for ubsec(4)
ok?
Index: ubsec.c
===
RCS file: /cvs/src/sys/dev/pci/ubsec.c,v
retrieving revision 1.161
diff -u -p -r1.161 ubsec.c
--- ubsec.c 10 Dec 2015 21:00:51 - 1.161
+++ ubsec.c 12 May 2016 10:54:57 -
@@ -1104,7 +1104,7 @@ ubsec_process(struct cryptop *crp)
q->q_dst_m = q->q_src_m;
q->q_dst_map = q->q_src_map;
} else {
- q->q_dst_m = m_copym2(q->q_src_m, 0, M_COPYALL,
+ q->q_dst_m = m_dup_pkt(q->q_src_m, 0,
M_NOWAIT);
if (q->q_dst_m == NULL) {
err = ENOMEM;
Re: pool related crashes, but "kernel did no panic"
Thank you!now that's a bug report.. On Thu, May 12, 2016 at 4:28 AM, Alexey Suslikov wrote: > On Wed, Apr 27, 2016 at 7:22 PM, Theo de Raadt > wrote: >>> On 27/04/16(Wed) 15:45, Alexey Suslikov wrote: >>> > Theo de Raadt cvs.openbsd.org> writes: >>> > >>> > > >>> > > Most of these bug reports completely stink. >>> > > >>> > > ALWAYS include *ALL* information in a report. >>> > >>> > In an idealistic world, yes. >>> >>> In an idealistic world their would be no bug. >> >> In an idealistic world, Alexey Suslikov wouldn't feel compelled to >> defend sloppiness. > > follow up is here > > http://marc.info/?l=openbsd-bugs&m=146304833425471&w=2 > http://marc.info/?l=openbsd-bugs&m=146304864925575&w=2 >
Iked binding
I have another VPN I need to setup, so it must be time for another attempt at trying to get iked to work. Has anyone already figured out a local hack to get iked to bind to a particular address so the packets from sendto() come with the correct source address on a multihomed system? I've poked around in ikev2_msg_send trying to add a bind() before the sendto but not really getting anywhere..
Re: make patch: more extensive sinclude support
This survived a full bulk build.
Now I'm fishing for okays.
(and a seeing eye)
On Tue, May 10, 2016 at 04:42:25PM +0200, Marc Espie wrote:
> Both bmake and gmake support a list of files in include/sinclude
> "systemV style".
>
> Adding this to our make would make us slightly more compatible.
>
> It also allows modern dependency patterns a la
>
> .sinclude ${SRC:R:=.d}
>
> Just went thru a full make build.
> could use a few eyes.
>
> The change is straightforward, but the patch a bit long, because I have
> to move the location of Var_Substs... keep it simple in the bsd make case (we
> don't want to create syntax by substituting variables). And so the handling
> of string intervals trickles down the chain of function.
>
> I refrained from also supporting gmake's glob extension, where they can do
> .sinclude *.d
>
> We have the match code in dir_expand.c, but this would be complicated to
> mesh with the include directory lookup in resolve_include_filename.
>
> Comments and okays welcome.
>
> Index: parse.c
> ===
> RCS file: /cvs/src/usr.bin/make/parse.c,v
> retrieving revision 1.115
> diff -u -p -r1.115 parse.c
> --- parse.c 22 Dec 2015 21:50:54 - 1.115
> +++ parse.c 10 May 2016 14:35:26 -
> @@ -149,7 +149,7 @@ static bool handle_undef(const char *);
> #define ParseReadLoopLine(linebuf) Parse_ReadUnparsedLine(linebuf, "for
> loop")
> static bool handle_bsd_command(Buffer, Buffer, const char *);
> static char *strip_comments(Buffer, const char *);
> -static char *resolve_include_filename(const char *, bool);
> +static char *resolve_include_filename(const char *, const char *, bool);
> static void handle_include_file(const char *, const char *, bool, bool);
> static bool lookup_bsd_include(const char *);
> static void lookup_sysv_style_include(const char *, const char *, bool);
> @@ -1081,13 +1081,13 @@ Parse_AddIncludeDir(const char*dir)
> }
>
> static char *
> -resolve_include_filename(const char *file, bool isSystem)
> +resolve_include_filename(const char *file, const char *efile, bool isSystem)
> {
> char *fullname;
>
> /* Look up system files on the system path first */
> if (isSystem) {
> - fullname = Dir_FindFileNoDot(file, systemIncludePath);
> + fullname = Dir_FindFileNoDoti(file, efile, systemIncludePath);
> if (fullname)
> return fullname;
> }
> @@ -1107,8 +1107,7 @@ resolve_include_filename(const char *fil
> if (slash != NULL) {
> char *newName;
>
> - newName = Str_concati(fname, slash, file,
> - strchr(file, '\0'), '/');
> + newName = Str_concati(fname, slash, file, efile, '/');
> fullname = Dir_FindFile(newName, userIncludePath);
> if (fullname == NULL)
> fullname = Dir_FindFile(newName, defaultPath);
> @@ -1121,10 +1120,10 @@ resolve_include_filename(const char *fil
> /* Now look first on the -I search path, then on the .PATH
>* search path, if not found in a -I directory.
>* XXX: Suffix specific? */
> - fullname = Dir_FindFile(file, userIncludePath);
> + fullname = Dir_FindFilei(file, efile, userIncludePath);
> if (fullname)
> return fullname;
> - fullname = Dir_FindFile(file, defaultPath);
> + fullname = Dir_FindFilei(file, efile, defaultPath);
> if (fullname)
> return fullname;
>
> @@ -1133,25 +1132,19 @@ resolve_include_filename(const char *fil
> if (isSystem)
> return NULL;
> else
> - return Dir_FindFile(file, systemIncludePath);
> + return Dir_FindFilei(file, efile, systemIncludePath);
> }
>
> static void
> -handle_include_file(const char *name, const char *ename, bool isSystem,
> +handle_include_file(const char *file, const char *efile, bool isSystem,
> bool errIfNotFound)
> {
> - char *file;
> char *fullname;
>
> - /* Substitute for any variables in the file name before trying to
> - * find the thing. */
> - file = Var_Substi(name, ename, NULL, false);
> -
> - fullname = resolve_include_filename(file, isSystem);
> + fullname = resolve_include_filename(file, efile, isSystem);
> if (fullname == NULL && errIfNotFound)
> - Parse_Error(PARSE_FATAL, "Could not find %s", file);
> - free(file);
> -
> + Parse_Error(PARSE_FATAL, "Could not find %s",
> + Str_dupi(file, efile));
>
> if (fullname != NULL) {
> FILE *f;
> @@ -1170,6 +1163,7 @@ lookup_bsd_include(const char *file)
> {
> char endc;
> const char *efile;
> + char *file2;
> bool isSystem;
>
> /* find starting delimiter */
> @@ -1197,30 +1191,48 @@ lookup_bsd_include(const char *file)
>
Supermicro X9SCM without ipmi panics while trying to attach ipmi0
It looks like the Supermicro X9SCM BIOS lies about the presence of a BMC.
This board does not have a BMC but OpenBSD 5.9 tries to attach it and fails
with the following panic:
...
acpibtn0 at acpi0: SLPB
acpibtn1 at acpi0: PWRB
panic: ipmi0: sendcmd fails
Starting stack trace...
panic() at panic+0x10b
ipmi_cmd_poll() at ipmi_cmd_poll+0x5c
ipmi_match() at ipmi_match+0x11f
config_scan() at config_scan+0x133
config_search() at config_search+0x129
config_found_sm() at config_found_sm+0x2b
mainbus_attach() at mainbus_attach+0x224
config_attach() at config_attach+0x1bc
cpu_configure() at cpu_configure+0x1b
main() at main+0x40d
end trace frame: 0x0, count: 4
OpenBSD 5.8 works fine though. I think the behaviour changed with rev1.84.
I also attached a workaround which is working for me.
Index: dev/ipmi.c
===
RCS file: /cvs/src/sys/dev/ipmi.c,v
retrieving revision 1.95
diff -u -p -u -p -r1.95 ipmi.c
--- dev/ipmi.c 11 Feb 2016 04:02:22 - 1.95
+++ dev/ipmi.c 12 May 2016 14:01:05 -
@@ -150,8 +150,8 @@ int get_sdr(struct ipmi_softc *, u_int16
intipmi_sendcmd(struct ipmi_cmd *);
intipmi_recvcmd(struct ipmi_cmd *);
-void ipmi_cmd(struct ipmi_cmd *);
-void ipmi_cmd_poll(struct ipmi_cmd *);
+intipmi_cmd(struct ipmi_cmd *);
+intipmi_cmd_poll(struct ipmi_cmd *);
void ipmi_cmd_wait(struct ipmi_cmd *);
void ipmi_cmd_wait_cb(void *);
@@ -1026,26 +1026,33 @@ ipmi_recvcmd(struct ipmi_cmd *c)
return (rc);
}
-void
+int
ipmi_cmd(struct ipmi_cmd *c)
{
+ int rv = 1;
+
if (cold || panicstr != NULL)
- ipmi_cmd_poll(c);
+ rv = ipmi_cmd_poll(c);
else
ipmi_cmd_wait(c);
+
+ return rv;
}
-void
+int
ipmi_cmd_poll(struct ipmi_cmd *c)
{
mtx_enter(&c->c_sc->sc_cmd_mtx);
if (ipmi_sendcmd(c)) {
- panic("%s: sendcmd fails", DEVNAME(c->c_sc));
+ mtx_leave(&c->c_sc->sc_cmd_mtx);
+ return 0; /* BIOS is lying, there is no BMC */
}
c->c_ccode = ipmi_recvcmd(c);
mtx_leave(&c->c_sc->sc_cmd_mtx);
+
+ return 1;
}
void
@@ -1671,10 +1678,11 @@ ipmi_match(struct device *parent, void *
c.c_maxrxlen = sizeof(cmd);
c.c_rxlen = 0;
c.c_data = cmd;
- ipmi_cmd(&c);
+ rv = ipmi_cmd(&c);
+
+ if (rv == 1) /* GETID worked, we got IPMI */
+ dbg_dump(1, "bmc data", c.c_rxlen, cmd);
- dbg_dump(1, "bmc data", c.c_rxlen, cmd);
- rv = 1; /* GETID worked, we got IPMI */
ipmi_unmap_regs(sc);
}
Re: [patch v3] cwm: Preserve stacking order during cycling
Quoth Okan Demirmen on Mon, Dec 14, 2015: > On Mon 2015.12.14 at 20:06 +0100, Matej Nanut wrote: > > Hello, > > > > will these patches eventually be commited to CVS? > > > > I really like this one. > > Hi - I haven't had spare cycles to look into this, nor the other recent > patches yet, but they have not been ignored. > > Thanks. Ping? -- Save energy: be apathetic.
Re: Supermicro X9SCM without ipmi panics while trying to attach ipmi0
That is among the reasons why ipmi is disabled.
And will remain disabled, until all the reasons are fixed.
> It looks like the Supermicro X9SCM BIOS lies about the presence of a BMC.
>
> This board does not have a BMC but OpenBSD 5.9 tries to attach it and fails
> with the following panic:
> ...
> acpibtn0 at acpi0: SLPB
> acpibtn1 at acpi0: PWRB
> panic: ipmi0: sendcmd fails
> Starting stack trace...
> panic() at panic+0x10b
> ipmi_cmd_poll() at ipmi_cmd_poll+0x5c
> ipmi_match() at ipmi_match+0x11f
> config_scan() at config_scan+0x133
> config_search() at config_search+0x129
> config_found_sm() at config_found_sm+0x2b
> mainbus_attach() at mainbus_attach+0x224
> config_attach() at config_attach+0x1bc
> cpu_configure() at cpu_configure+0x1b
> main() at main+0x40d
> end trace frame: 0x0, count: 4
>
> OpenBSD 5.8 works fine though. I think the behaviour changed with rev1.84.
> I also attached a workaround which is working for me.
>
>
> Index: dev/ipmi.c
> ===
> RCS file: /cvs/src/sys/dev/ipmi.c,v
> retrieving revision 1.95
> diff -u -p -u -p -r1.95 ipmi.c
> --- dev/ipmi.c11 Feb 2016 04:02:22 - 1.95
> +++ dev/ipmi.c12 May 2016 14:01:05 -
> @@ -150,8 +150,8 @@ int get_sdr(struct ipmi_softc *, u_int16
>
> int ipmi_sendcmd(struct ipmi_cmd *);
> int ipmi_recvcmd(struct ipmi_cmd *);
> -void ipmi_cmd(struct ipmi_cmd *);
> -void ipmi_cmd_poll(struct ipmi_cmd *);
> +int ipmi_cmd(struct ipmi_cmd *);
> +int ipmi_cmd_poll(struct ipmi_cmd *);
> void ipmi_cmd_wait(struct ipmi_cmd *);
> void ipmi_cmd_wait_cb(void *);
>
> @@ -1026,26 +1026,33 @@ ipmi_recvcmd(struct ipmi_cmd *c)
> return (rc);
> }
>
> -void
> +int
> ipmi_cmd(struct ipmi_cmd *c)
> {
> + int rv = 1;
> +
> if (cold || panicstr != NULL)
> - ipmi_cmd_poll(c);
> + rv = ipmi_cmd_poll(c);
> else
> ipmi_cmd_wait(c);
> +
> + return rv;
> }
>
> -void
> +int
> ipmi_cmd_poll(struct ipmi_cmd *c)
> {
> mtx_enter(&c->c_sc->sc_cmd_mtx);
>
> if (ipmi_sendcmd(c)) {
> - panic("%s: sendcmd fails", DEVNAME(c->c_sc));
> + mtx_leave(&c->c_sc->sc_cmd_mtx);
> + return 0; /* BIOS is lying, there is no BMC */
> }
> c->c_ccode = ipmi_recvcmd(c);
>
> mtx_leave(&c->c_sc->sc_cmd_mtx);
> +
> + return 1;
> }
>
> void
> @@ -1671,10 +1678,11 @@ ipmi_match(struct device *parent, void *
> c.c_maxrxlen = sizeof(cmd);
> c.c_rxlen = 0;
> c.c_data = cmd;
> - ipmi_cmd(&c);
> + rv = ipmi_cmd(&c);
> +
> + if (rv == 1) /* GETID worked, we got IPMI */
> + dbg_dump(1, "bmc data", c.c_rxlen, cmd);
>
> - dbg_dump(1, "bmc data", c.c_rxlen, cmd);
> - rv = 1; /* GETID worked, we got IPMI */
> ipmi_unmap_regs(sc);
> }
>
Re: make patch: more extensive sinclude support
On Tue, 10 May 2016 16:42:25 +0200, Marc Espie wrote:
> Both bmake and gmake support a list of files in include/sinclude
> "systemV style".
>
> Adding this to our make would make us slightly more compatible.
>
> It also allows modern dependency patterns a la
>
> .sinclude ${SRC:R:=.d}
>
> Just went thru a full make build.
> could use a few eyes.
>
> The change is straightforward, but the patch a bit long, because I have
> to move the location of Var_Substs... keep it simple in the bsd make case (we
> don't want to create syntax by substituting variables). And so the handling
> of string intervals trickles down the chain of function.
>
> I refrained from also supporting gmake's glob extension, where they can do
> .sinclude *.d
>
> We have the match code in dir_expand.c, but this would be complicated to
> mesh with the include directory lookup in resolve_include_filename.
>
> Comments and okays welcome.
Looks OK but one minor comment:
> Index: parse.c
> ===
> RCS file: /cvs/src/usr.bin/make/parse.c,v
> retrieving revision 1.115
> diff -u -p -r1.115 parse.c
> --- parse.c 22 Dec 2015 21:50:54 - 1.115
> +++ parse.c 10 May 2016 14:35:26 -
> @@ -1133,25 +1132,19 @@ resolve_include_filename(const char *fil
> if (isSystem)
> return NULL;
> else
> - return Dir_FindFile(file, systemIncludePath);
> + return Dir_FindFilei(file, efile, systemIncludePath);
> }
>
> static void
> -handle_include_file(const char *name, const char *ename, bool isSystem,
> +handle_include_file(const char *file, const char *efile, bool isSystem,
> bool errIfNotFound)
> {
> - char *file;
> char *fullname;
>
> - /* Substitute for any variables in the file name before trying to
> - * find the thing. */
> - file = Var_Substi(name, ename, NULL, false);
> -
> - fullname = resolve_include_filename(file, isSystem);
> + fullname = resolve_include_filename(file, efile, isSystem);
> if (fullname == NULL && errIfNotFound)
> - Parse_Error(PARSE_FATAL, "Could not find %s", file);
> - free(file);
> -
> + Parse_Error(PARSE_FATAL, "Could not find %s",
> + Str_dupi(file, efile));
I don't think you need that Str_dupi() if you do:
Parse_Error(PARSE_FATAL, "Could not find %.*s",
(int)(efile - file), file);
> if (fullname != NULL) {
> FILE *f;
remove kevent perm check
There is a permission check for EVFILT_PROC that is not documented. Actually, it directly contradicts the documentation, which says you can watch any process you can see. The documented behavior makes sense, since I could also just run ps in a tight loop and get the same info, only less efficiently. The check was added without note during the initial import (as suser only!) and then refined shortly later, but no justification given. I think we should remove the check. It doesn't make sense, and it's different from other systems using kqueue. (I also had to work around it in rebound, where some code could be organized better if it weren't for the need to call kevent before switching IDs.) Index: kern_event.c === RCS file: /cvs/src/sys/kern/kern_event.c,v retrieving revision 1.71 diff -u -p -r1.71 kern_event.c --- kern_event.c6 Jan 2016 17:58:46 - 1.71 +++ kern_event.c12 May 2016 16:01:20 - @@ -219,15 +219,6 @@ filt_procattach(struct knote *kn) if (pr->ps_flags & PS_EXITING) return (ESRCH); - /* -* Fail if it's not owned by you, or the last exec gave us -* setuid/setgid privs (unless you're root). -*/ - if (pr != curproc->p_p && - (pr->ps_ucred->cr_ruid != curproc->p_ucred->cr_ruid || - (pr->ps_flags & PS_SUGID)) && suser(curproc, 0) != 0) - return (EACCES); - kn->kn_ptr.p_process = pr; kn->kn_flags |= EV_CLEAR; /* automatically set */
Re: remove kevent perm check
On Thu, 12 May 2016 12:07:43 -0400, "Ted Unangst" wrote: > I think we should remove the check. It doesn't make sense, and it's different > from other systems using kqueue. (I also had to work around it in rebound, > where some code could be organized better if it weren't for the need to call > kevent before switching IDs.) FreeBSD has process visibility controls and checks them in this location. We don't have any such controls, so removing that chunk should be fine. OK millert@ - todd
Re: remove kevent perm check
> > I think we should remove the check. It doesn't make sense, and it's > > different > > from other systems using kqueue. (I also had to work around it in rebound, > > where some code could be organized better if it weren't for the need to call > > kevent before switching IDs.) > > FreeBSD has process visibility controls and checks them in this > location. We don't have any such controls, so removing that chunk > should be fine. OK millert@ Unless we have someone who wants to go down that road...
Re: remove kevent perm check
> > > I think we should remove the check. It doesn't make sense, and it's > > > different > > > from other systems using kqueue. (I also had to work around it in rebound, > > > where some code could be organized better if it weren't for the need to > > > call > > > kevent before switching IDs.) > > > > FreeBSD has process visibility controls and checks them in this > > location. We don't have any such controls, so removing that chunk > > should be fine. OK millert@ > > Unless we have someone who wants to go down that road... Or, should this specific bit be disallowed if pledge'd, but lacking "proc"?
Re: remove kevent perm check
Theo de Raadt wrote:
> > > > I think we should remove the check. It doesn't make sense, and it's
> > > > different
> > > > from other systems using kqueue. (I also had to work around it in
> > > > rebound,
> > > > where some code could be organized better if it weren't for the need to
> > > > call
> > > > kevent before switching IDs.)
> > >
> > > FreeBSD has process visibility controls and checks them in this
> > > location. We don't have any such controls, so removing that chunk
> > > should be fine. OK millert@
> >
> > Unless we have someone who wants to go down that road...
>
> Or, should this specific bit be disallowed if pledge'd, but lacking "proc"?
That is consistent.
Index: kern_event.c
===
RCS file: /cvs/src/sys/kern/kern_event.c,v
retrieving revision 1.71
diff -u -p -r1.71 kern_event.c
--- kern_event.c6 Jan 2016 17:58:46 - 1.71
+++ kern_event.c12 May 2016 17:32:05 -
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -211,6 +212,10 @@ filt_procattach(struct knote *kn)
{
struct process *pr;
+ if ((curproc->p_p->ps_flags & PS_PLEDGE) &&
+ (curproc->p_p->ps_pledge & PLEDGE_PROC) == 0)
+ return pledge_fail(curproc, EPERM, PLEDGE_PROC);
+
pr = prfind(kn->kn_id);
if (pr == NULL)
return (ESRCH);
@@ -218,15 +223,6 @@ filt_procattach(struct knote *kn)
/* exiting processes can't be specified */
if (pr->ps_flags & PS_EXITING)
return (ESRCH);
-
- /*
-* Fail if it's not owned by you, or the last exec gave us
-* setuid/setgid privs (unless you're root).
-*/
- if (pr != curproc->p_p &&
- (pr->ps_ucred->cr_ruid != curproc->p_ucred->cr_ruid ||
- (pr->ps_flags & PS_SUGID)) && suser(curproc, 0) != 0)
- return (EACCES);
kn->kn_ptr.p_process = pr;
kn->kn_flags |= EV_CLEAR; /* automatically set */
fsync option for install
install has a "safe mode" -S option, although it's not as entirely safe as one
might assume. It relies on rename() being an atomic operation, which is good.
However, rename doesn't guarantee that a file's *contents* are on disk. Thus,
there is a window between the rename and the eventual flushing of the file's
data buffers where a crash will leave you with an incomplete file upon reboot.
Calling fsync() by default may be too expensive, and is probably unnecessary
for many files. But for some files, like libc, we may wish to be certain the
new file is committed before doing the rename. This adds a -F option to call
fsync.
Index: install.1
===
RCS file: /cvs/src/usr.bin/xinstall/install.1,v
retrieving revision 1.27
diff -u -p -r1.27 install.1
--- install.1 1 Feb 2016 15:54:58 - 1.27
+++ install.1 12 May 2016 19:50:56 -
@@ -38,7 +38,7 @@
.Nd install binaries
.Sh SYNOPSIS
.Nm install
-.Op Fl bCcDdpSs
+.Op Fl bCcDFdpSs
.Op Fl B Ar suffix
.Op Fl f Ar flags
.Op Fl g Ar group
@@ -105,6 +105,10 @@ This option cannot be used with the
or
.Fl s
options.
+.It Fl F
+Use
+.Xr fsync 2
+to ensure the created file is saved to disk.
.It Fl f Ar flags
Specify the target's file
.Ar flags .
Index: xinstall.c
===
RCS file: /cvs/src/usr.bin/xinstall/xinstall.c,v
retrieving revision 1.63
diff -u -p -r1.63 xinstall.c
--- xinstall.c 31 Dec 2015 16:16:54 - 1.63
+++ xinstall.c 12 May 2016 19:49:27 -
@@ -61,7 +61,7 @@
struct passwd *pp;
struct group *gp;
-int dobackup, docompare, dodest, dodir, dopreserve, dostrip, safecopy;
+int dobackup, docompare, dodest, dodir, dofsync, dopreserve, dostrip, safecopy;
int mode = S_IRWXU|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH;
char pathbuf[PATH_MAX], tempfile[PATH_MAX];
char *suffix = BACKUP_SUFFIX;
@@ -90,7 +90,7 @@ main(int argc, char *argv[])
char *flags, *to_name, *group = NULL, *owner = NULL;
iflags = 0;
- while ((ch = getopt(argc, argv, "B:bCcDdf:g:m:o:pSs")) != -1)
+ while ((ch = getopt(argc, argv, "B:bCcDdFf:g:m:o:pSs")) != -1)
switch(ch) {
case 'C':
docompare = 1;
@@ -104,6 +104,9 @@ main(int argc, char *argv[])
case 'c':
/* For backwards compatibility. */
break;
+ case 'F':
+ dofsync = 1;
+ break;
case 'f':
flags = optarg;
if (strtofflags(&flags, &fset, NULL))
@@ -377,6 +380,8 @@ install(char *from_name, char *to_name,
safecopy ? tempfile :to_name, strerror(errno));
}
+ if (dofsync)
+ fsync(to_fd);
(void)close(to_fd);
if (!devnull)
(void)close(from_fd);
@@ -618,7 +623,7 @@ void
usage(void)
{
(void)fprintf(stderr, "\
-usage: install [-bCcDdpSs] [-B suffix] [-f flags] [-g group] [-m mode] [-o
owner]\n source ... target ...\n");
+usage: install [-bCcDdFpSs] [-B suffix] [-f flags] [-g group] [-m mode] [-o
owner]\n source ... target ...\n");
exit(1);
/* NOTREACHED */
}
Re: smu(4) PWM Fan Support
On Wed, May 11, 2016 at 07:53:56PM +, Miod Vallat wrote: > > > I've recently noticed that two of five fans in my G5 don't spin up. > > That's because smu(4) currently just supports RPM fans. > > The attached diff adds initial support for PWM fans as well. > > fcu(4) could benefit from a similar change, as there may be pwm fans > there too. At least RackMac3,1 has two pwm fans according to the eeprom > -p output. Hmm, looking at the fcu(4) code it already should support pwm fans, no?
Re: smu(4) PWM Fan Support
On Tue, May 10, 2016 at 11:46:24PM +0200, Marcus Glocker wrote:
> I've recently noticed that two of five fans in my G5 don't spin up.
> That's because smu(4) currently just supports RPM fans.
> The attached diff adds initial support for PWM fans as well.
>
> sysctl before:
> # sysctl -a | grep fan
> hw.sensors.smu0.fan0=994 RPM (Rear Fan 0)
> hw.sensors.smu0.fan1=994 RPM (Rear fan 1)
> hw.sensors.smu0.fan2=994 RPM (Front Fan)
>
> sysctl after:
> # sysctl -a | grep fan
> hw.sensors.smu0.fan0=994 RPM (Rear Fan 0)
> hw.sensors.smu0.fan1=994 RPM (Rear fan 1)
> hw.sensors.smu0.fan2=994 RPM (Front Fan)
> hw.sensors.smu0.fan3=589 RPM (Slots Fan)
> hw.sensors.smu0.fan4=589 RPM (Drive Bay)
>
> I was first thinking to introduce a new sensor type in sys/sensor.h
> called SENSOR_FANPWM, but finally I think it's more intuitive to
> display the RPM value for all fans in general.
Alternatively here a diff which displays the fan pwm instead of the rpm.
Maybe this is more straight forward ...
> In case this makes it in I would like to split the RPM read which
> is currently done in smu_fan_refresh() in a own function as next,
> same as for the PWM read. Also with the background that there seems
> to be another set/read method for new style fans which may fail
> currently which we could implement.
Index: sys/arch/macppc/dev/smu.c
===
RCS file: /cvs/src/sys/arch/macppc/dev/smu.c,v
retrieving revision 1.28
diff -u -p -u -p -r1.28 smu.c
--- sys/arch/macppc/dev/smu.c 4 May 2016 08:20:58 - 1.28
+++ sys/arch/macppc/dev/smu.c 12 May 2016 20:09:04 -
@@ -44,6 +44,9 @@ struct smu_fan {
u_int16_t min_rpm;
u_int16_t max_rpm;
u_int16_t unmanaged_rpm;
+ u_int16_t min_pwm;
+ u_int16_t max_pwm;
+ u_int16_t unmanaged_pwm;
struct ksensor sensor;
};
@@ -144,6 +147,9 @@ int smu_time_read(time_t *);
intsmu_time_write(time_t);
intsmu_get_datablock(struct smu_softc *sc, u_int8_t, u_int8_t *, size_t);
intsmu_fan_set_rpm(struct smu_softc *, struct smu_fan *, u_int16_t);
+intsmu_fan_set_pwm(struct smu_softc *, struct smu_fan *, u_int16_t);
+intsmu_fan_read_pwm(struct smu_softc *, struct smu_fan *, u_int16_t *,
+ u_int16_t *);
intsmu_fan_refresh(struct smu_softc *, struct smu_fan *);
intsmu_sensor_refresh(struct smu_softc *, struct smu_sensor *);
void smu_refresh_sensors(void *);
@@ -250,7 +256,7 @@ smu_attach(struct device *parent, struct
time_read = smu_time_read;
time_write = smu_time_write;
- /* Fans */
+ /* RPM Fans */
node = OF_getnodebyname(ca->ca_node, "rpm-fans");
if (node == 0)
node = OF_getnodebyname(ca->ca_node, "fans");
@@ -260,7 +266,7 @@ smu_attach(struct device *parent, struct
continue;
if (strcmp(type, "fan-rpm-control") != 0) {
- printf(": unsupported fan type: %s\n", type);
+ printf(": unsupported rpm-fan type: %s\n", type);
return;
}
@@ -299,6 +305,53 @@ smu_attach(struct device *parent, struct
#endif
}
+ /* PWM Fans */
+ node = OF_getnodebyname(ca->ca_node, "pwm-fans");
+ for (node = OF_child(node); node; node = OF_peer(node)) {
+ if (OF_getprop(node, "reg", ®, sizeof reg) <= 0 ||
+ OF_getprop(node, "device_type", type, sizeof type) <= 0)
+ continue;
+
+ if (strcmp(type, "fan-pwm-control") != 0) {
+ printf(": unsupported pwm-fan type: %s\n", type);
+ return;
+ }
+
+ if (sc->sc_num_fans >= SMU_MAXFANS) {
+ printf(": too many fans\n");
+ return;
+ }
+
+ fan = &sc->sc_fans[sc->sc_num_fans++];
+ fan->sensor.type = SENSOR_PERCENT;
+ fan->sensor.flags = SENSOR_FINVALID;
+ fan->reg = reg;
+
+ if (OF_getprop(node, "min-value", &val, sizeof val) <= 0)
+ val = 0;
+ fan->min_pwm = val;
+ if (OF_getprop(node, "max-value", &val, sizeof val) <= 0)
+ val = 0x;
+ fan->max_pwm = val;
+ if (OF_getprop(node, "unmanage-value", &val, sizeof val) > 0)
+ fan->unmanaged_pwm = val;
+ else if (OF_getprop(node, "safe-value", &val, sizeof val) > 0)
+ fan->unmanaged_pwm = val;
+ else
+ fan->unmanaged_pwm = fan->min_pwm;
+
+ if (OF_getprop(node, "location", loc, sizeof loc) <= 0)
+ strlcpy(loc, "Unknown", sizeof loc);
+ strlcpy(fan->sensor.desc, loc, sizeof sensor->sensor.desc);
+
+ /* Start running f
maxds(4) return on attach error
On my G5 the maxds(4) control register read fails. I guess because
there is no such chip on the mainboard though the eeprom -p claims
there is one (or it's broken).
In case maxds(4) can't read the control register at attach time
we should return (like lmtemp(4) does it) instead of continue to
setup a task which will continue to fail as well on every refresh
call.
Index: sys/dev/i2c/ds1631.c
===
RCS file: /cvs/src/sys/dev/i2c/ds1631.c,v
retrieving revision 1.11
diff -u -p -u -p -r1.11 ds1631.c
--- sys/dev/i2c/ds1631.c12 Aug 2009 17:13:30 - 1.11
+++ sys/dev/i2c/ds1631.c11 May 2016 18:50:11 -
@@ -108,6 +108,10 @@ dostart:
sc->sc_addr, &cmd, sizeof cmd, NULL, 0, 0);
printf(", starting");
}
+ } else {
+ iic_release_bus(sc->sc_tag, 0);
+ printf(", fails to respond\n");
+ return;
}
iic_release_bus(sc->sc_tag, 0);
Re: smu(4) PWM Fan Support
> > fcu(4) could benefit from a similar change, as there may be pwm fans > > there too. At least RackMac3,1 has two pwm fans according to the eeprom > > -p output. > > Hmm, looking at the fcu(4) code it already should support pwm fans, no? Doh! I was looking for `fan' in the sensors output and did not notice `percent'. Sorry for the noise.
Re: maxds(4) return on attach error
> Date: Thu, 12 May 2016 22:31:37 +0200
> From: Marcus Glocker
>
> On my G5 the maxds(4) control register read fails. I guess because
> there is no such chip on the mainboard though the eeprom -p claims
> there is one (or it's broken).
>
> In case maxds(4) can't read the control register at attach time
> we should return (like lmtemp(4) does it) instead of continue to
> setup a task which will continue to fail as well on every refresh
> call.
Committed. Still need to look at your smu(4) diffs.
> Index: sys/dev/i2c/ds1631.c
> ===
> RCS file: /cvs/src/sys/dev/i2c/ds1631.c,v
> retrieving revision 1.11
> diff -u -p -u -p -r1.11 ds1631.c
> --- sys/dev/i2c/ds1631.c 12 Aug 2009 17:13:30 - 1.11
> +++ sys/dev/i2c/ds1631.c 11 May 2016 18:50:11 -
> @@ -108,6 +108,10 @@ dostart:
> sc->sc_addr, &cmd, sizeof cmd, NULL, 0, 0);
> printf(", starting");
> }
> + } else {
> + iic_release_bus(sc->sc_tag, 0);
> + printf(", fails to respond\n");
> + return;
> }
>
> iic_release_bus(sc->sc_tag, 0);
>
>
Re: smu(4) PWM Fan Support
On Thu, May 12, 2016 at 08:39:01PM +, Miod Vallat wrote: > > > fcu(4) could benefit from a similar change, as there may be pwm fans > > > there too. At least RackMac3,1 has two pwm fans according to the eeprom > > > -p output. > > > > Hmm, looking at the fcu(4) code it already should support pwm fans, no? > > Doh! I was looking for `fan' in the sensors output and did not notice > `percent'. Sorry for the noise. :-) That's exactly the reason why the first diff uses the rpm value in favour of the pwm percentage value. It's more intuitive to expect all fans to show up as a fan sensor type instead of a percentage type. But I guess since other drivers also use the percentage value for pwm fans it's more strict to keep this, hence the second diff.
Re: maxds(4) return on attach error
On Thu, May 12, 2016 at 11:00:54PM +0200, Mark Kettenis wrote: > > Date: Thu, 12 May 2016 22:31:37 +0200 > > From: Marcus Glocker > > > > On my G5 the maxds(4) control register read fails. I guess because > > there is no such chip on the mainboard though the eeprom -p claims > > there is one (or it's broken). > > > > In case maxds(4) can't read the control register at attach time > > we should return (like lmtemp(4) does it) instead of continue to > > setup a task which will continue to fail as well on every refresh > > call. > > Committed. Still need to look at your smu(4) diffs. Ok, thanks!
Re: pool related crashes, but "kernel did no panic"
On Fri, May 13, 2016 at 3:59 AM, David Gwynne wrote: > >> On 12 May 2016, at 20:28, Alexey Suslikov wrote: >> >> On Wed, Apr 27, 2016 at 7:22 PM, Theo de Raadt >> wrote: On 27/04/16(Wed) 15:45, Alexey Suslikov wrote: > Theo de Raadt cvs.openbsd.org> writes: > >> >> Most of these bug reports completely stink. >> >> ALWAYS include *ALL* information in a report. > > In an idealistic world, yes. In an idealistic world their would be no bug. >>> >>> In an idealistic world, Alexey Suslikov wouldn't feel compelled to >>> defend sloppiness. >> >> follow up is here >> >> http://marc.info/?l=openbsd-bugs&m=146304833425471&w=2 >> http://marc.info/?l=openbsd-bugs&m=146304864925575&w=2 >> > > this shoudl be fixed in stable. can you make sure you have the following: > > http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/kern/uipc_mbuf.c.diff?r1=1.219&r2=1.219.2.1 what do you think about this (new) one http://marc.info/?l=openbsd-bugs&m=146312050712969&w=2 I really can do more to debug this and asked for an advice from the begging of this thread.
