fgen free alias in error path
Hi All,
From NetBSD:
Coverity CID 1748: Free alias on error.
alias-name = strdup(token-text);
if (alias-name == NULL)
(void)err(1, out of memory);
token = yylex();
if (token == NULL) {
free(alias);
(void)printf( EOF in alias definition\n);
return;
alias-name calls strdup, but it doesn't free it on if (token == NULL).
Shouldn't it be free'ed as well ?
Index: src/usr.bin/fgen/fgen.l
===
RCS file: /cvs/src/usr.bin/fgen/fgen.l,v
retrieving revision 1.9
diff -u -p -r1.9 fgen.l
--- src/usr.bin/fgen/fgen.l 10 Dec 2009 17:31:49 - 1.9
+++ src/usr.bin/fgen/fgen.l 30 Dec 2013 08:03:10 -
@@ -1232,6 +1232,7 @@ tokenize(input)
token = yylex();
if (token == NULL) {
+ free(alias-name);
free(alias);
(void)printf( EOF in alias definition\n);
return;
irc.mindcry.org down
Hi All, I can no longer find irc.mindcry.org on the internet. Is that permanent or temporary ? //logan c-x-c-c
Re: cmp fd leak fix
On Sun, Dec 29, 2013 at 23:28 -0800, Loganaden Velvindron wrote: Hi All, From NetBSD: Plug fd leak. Coverity CID 1624. fd leak? they're leaking back to the system. the function is not recursive nor it's called in a loop. Index: src/usr.bin/cmp/special.c === RCS file: /cvs/src/usr.bin/cmp/special.c,v retrieving revision 1.7 diff -u -p -r1.7 special.c --- src/usr.bin/cmp/special.c 19 Jan 2011 13:01:25 - 1.7 +++ src/usr.bin/cmp/special.c 30 Dec 2013 06:54:05 - @@ -88,6 +88,8 @@ eof:if (ferror(fp1)) } else if (feof(fp2)) eofmsg(file2); + (void)fclose(fp1); + (void)fclose(fp2); if (dfound) exit(DIFF_EXIT); }
Re: column memory leak fix
On Sun, Dec 29, 2013 at 22:45 -0800, Loganaden Velvindron wrote: Hi All, From NetBSD: Plug memory leak. Coverity CID 1596 memory leak? can you please elaborate where else this memory is leaking if not back to the system. Index: src/usr.bin/column/column.c === RCS file: /cvs/src/usr.bin/column/column.c,v retrieving revision 1.16 diff -u -p -r1.16 column.c --- src/usr.bin/column/column.c 26 Nov 2013 13:18:55 - 1.16 +++ src/usr.bin/column/column.c 30 Dec 2013 06:38:02 - @@ -241,6 +241,9 @@ maketbl(void) (void)printf(%s\n, t-list[coloff]); } } + free(tbl); + free(cols); + free(lens); } #define DEFNUM 1000
Re: column memory leak fix
On Mon, Dec 30, 2013 at 12:45:47PM +0100, Mike Belopuhov wrote: On Sun, Dec 29, 2013 at 22:45 -0800, Loganaden Velvindron wrote: Hi All, From NetBSD: Plug memory leak. Coverity CID 1596 memory leak? can you please elaborate where else this memory is leaking if not back to the system. After a short discussion on IRC, and some digging, it makes sense that the system cleanups up automatically, and therefore it is not necessary. Index: src/usr.bin/column/column.c === RCS file: /cvs/src/usr.bin/column/column.c,v retrieving revision 1.16 diff -u -p -r1.16 column.c --- src/usr.bin/column/column.c 26 Nov 2013 13:18:55 - 1.16 +++ src/usr.bin/column/column.c 30 Dec 2013 06:38:02 - @@ -241,6 +241,9 @@ maketbl(void) (void)printf(%s\n, t-list[coloff]); } } + free(tbl); + free(cols); + free(lens); } #defineDEFNUM 1000
TOD clock for octeons
The following enables the DS1337 RTC clock found on octeon boards.
I've tested it on my DSR-500 and would love to hear about tests on other
machines.
Comments? Okays?
Index: conf/GENERIC
===
RCS file: /cvs/src/sys/arch/octeon/conf/GENERIC,v
retrieving revision 1.11
diff -u -p -r1.11 GENERIC
--- conf/GENERIC24 Oct 2013 20:47:08 - 1.11
+++ conf/GENERIC30 Dec 2013 12:42:02 -
@@ -31,6 +31,8 @@ clock0at mainbus0
iobus0 at mainbus0
uartbus0 at mainbus0
+octrtc0at mainbus0
+
octcf0 at iobus0
octrng0at iobus0
Index: conf/files.octeon
===
RCS file: /cvs/src/sys/arch/octeon/conf/files.octeon,v
retrieving revision 1.16
diff -u -p -r1.16 files.octeon
--- conf/files.octeon 4 Nov 2013 14:07:16 - 1.16
+++ conf/files.octeon 30 Dec 2013 12:42:02 -
@@ -48,6 +48,11 @@ attach cpu at mainbus
device clock
attach clock at mainbus
+# TOD cloc
+device octrtc
+attach octrtc at mainbus
+file arch/octeon/dev/octrtc.coctrtc
+
define iobus {[base = -1]}
device iobus
attach iobus at mainbus
Index: dev/octrtc.c
===
RCS file: dev/octrtc.c
diff -N dev/octrtc.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ dev/octrtc.c30 Dec 2013 12:42:03 -
@@ -0,0 +1,218 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2013 Paul Irofti.
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include sys/param.h
+#include sys/systm.h
+#include sys/device.h
+#include sys/proc.h
+#include sys/timetc.h
+
+#include machine/bus.h
+#include machine/autoconf.h
+#include machine/octeonvar.h
+
+#ifdef OCTRTC_DEBUG/* VERY VERBOSE */
+#define DPRINTF(x) printf x
+#else
+#define DPRINTF(x)
+#endif
+
+#define MIO_TWS_SW_TWSI0x000118001000ULL
+#define OCTRTC_REG 0x68
+
+struct octrtc_softc {
+ struct device sc_dev;
+};
+
+struct cfdriver octrtc_cd = {
+ NULL, octrtc, DV_DULL
+};
+
+intoctrtc_match(struct device *, void *, void *);
+void octrtc_attach(struct device *, struct device *, void *);
+
+u_int octrtc_gettime(struct timecounter *);
+intoctrtc_read(uint8_t *, char);
+
+struct cfattach octrtc_ca = {
+ sizeof(struct octrtc_softc), octrtc_match, octrtc_attach,
+};
+
+
+static struct timecounter octrtc_timecounter = {
+ octrtc_gettime, /* get_timecount */
+ 0, /* no poll_pps */
+ 0x, /* counter_mask */
+ 10, /* frequency */
+ DS1337, /* name */
+ 1000, /* quality */
+};
+
+
+union mio_tws_sw_twsi_reg {
+ uint64_t reg;
+ struct cvmx_mio_twsx_sw_twsi_s {
+ uint64_t v:1; /* Valid bit */
+ uint64_t slonly:1; /* Slave Only Mode */
+ uint64_t eia:1; /* Extended Internal Address */
+ uint64_t op:4; /* Opcode field */
+ uint64_t r:1; /* Read bit or result */
+ uint64_t sovr:1;/* Size Override */
+ uint64_t size:3;/* Size in bytes */
+ uint64_t scr:2; /* Scratch, unused */
+ uint64_t a:10; /* Address field */
+ uint64_t ia:5; /* Internal Address */
+ uint64_t eop_ia:3; /* Extra opcode */
+ uint64_t d:32; /* Data Field */
+ } field;
+};
+
+
+static inline int bcd2bin(int);
+static inline int
+bcd2bin(int datum)
+{
+ return (datum 4) * 10 + (datum 0x0f);
+}
+
+int
+octrtc_match(struct device *parent, void *match, void *aux)
+{
+ return 1;
+}
+
+void
+octrtc_attach(struct device *parent, struct device *self, void *aux)
+{
+ struct octrtc_softc *sc = (struct octrtc_softc *)self;
+
+ octrtc_timecounter.tc_priv = sc;
+ tc_init(octrtc_timecounter);
+
+ printf(: DS1337\n);
+}
+
+u_int
+octrtc_gettime(struct timecounter *tc)
+{
+ uint8_t tod[8];
+ uint8_t check;
+ int i, rc;
+
+ int nretries = 2;
+
+
Re: column memory leak fix
On Mon, Dec 30, 2013 at 03:59 -0800, Loganaden Velvindron wrote: On Mon, Dec 30, 2013 at 12:45:47PM +0100, Mike Belopuhov wrote: On Sun, Dec 29, 2013 at 22:45 -0800, Loganaden Velvindron wrote: Hi All, From NetBSD: Plug memory leak. Coverity CID 1596 memory leak? can you please elaborate where else this memory is leaking if not back to the system. After a short discussion on IRC, and some digging, it makes sense that the system cleanups up automatically, and therefore it is not necessary. the patch can be committed though for the sake of better style. Index: src/usr.bin/column/column.c === RCS file: /cvs/src/usr.bin/column/column.c,v retrieving revision 1.16 diff -u -p -r1.16 column.c --- src/usr.bin/column/column.c 26 Nov 2013 13:18:55 - 1.16 +++ src/usr.bin/column/column.c 30 Dec 2013 06:38:02 - @@ -241,6 +241,9 @@ maketbl(void) (void)printf(%s\n, t-list[coloff]); } } + free(tbl); + free(cols); + free(lens); } #define DEFNUM 1000
Re: column memory leak fix
On Mon, Dec 30, 2013 at 13:53, Mike Belopuhov wrote: On Mon, Dec 30, 2013 at 03:59 -0800, Loganaden Velvindron wrote: On Mon, Dec 30, 2013 at 12:45:47PM +0100, Mike Belopuhov wrote: On Sun, Dec 29, 2013 at 22:45 -0800, Loganaden Velvindron wrote: Hi All, From NetBSD: Plug memory leak. Coverity CID 1596 memory leak? can you please elaborate where else this memory is leaking if not back to the system. After a short discussion on IRC, and some digging, it makes sense that the system cleanups up automatically, and therefore it is not necessary. the patch can be committed though for the sake of better style. but please review to make sure they are correct.
Re: Typo in nfs_boot.c
2013/9/12 Eivind Evensen eivi...@terraplane.org: Hello. Trying to figure out what I've done wrong to have a diskless setup boot the kernel and then try to talk to a broadcast address rather than the nfsserver, I saw this typo. Eivind Hi, I think I have the same problem as, when booting a client, I get RPC timeout for server 192.168.100.255 how did you solve the problem of talking to a broadcast address? or, what am I doing wrong? Thanks -- Matteo Filippetto http://www.op83.eu
Re: column memory leak fix
On Mon, Dec 30, 2013 at 08:42:00AM -0500, Ted Unangst wrote: On Mon, Dec 30, 2013 at 13:53, Mike Belopuhov wrote: On Mon, Dec 30, 2013 at 03:59 -0800, Loganaden Velvindron wrote: On Mon, Dec 30, 2013 at 12:45:47PM +0100, Mike Belopuhov wrote: On Sun, Dec 29, 2013 at 22:45 -0800, Loganaden Velvindron wrote: Hi All, From NetBSD: Plug memory leak. Coverity CID 1596 memory leak? can you please elaborate where else this memory is leaking if not back to the system. After a short discussion on IRC, and some digging, it makes sense that the system cleanups up automatically, and therefore it is not necessary. the patch can be committed though for the sake of better style. but please review to make sure they are correct. It would be great if the ldconfig, and the various maintainers could review the diff, and help me improve the diffs, or discard them. I'm mostly interested in finding the small security issues and fixing them, rather than fixing style issues :-)
Re: Typo in nfs_boot.c
2013/12/30 matteo filippetto matteo.filippe...@gmail.com: 2013/9/12 Eivind Evensen eivi...@terraplane.org: Hello. Trying to figure out what I've done wrong to have a diskless setup boot the kernel and then try to talk to a broadcast address rather than the nfsserver, I saw this typo. Eivind Hi, I think I have the same problem as, when booting a client, I get RPC timeout for server 192.168.100.255 how did you solve the problem of talking to a broadcast address? or, what am I doing wrong? Thanks Ok, just solved the problem: start mountd at boot. Regards -- Matteo Filippetto http://www.op83.eu
Re: column memory leak fix
On 30 December 2013 16:35, Loganaden Velvindron lo...@elandsys.com wrote: On Mon, Dec 30, 2013 at 08:42:00AM -0500, Ted Unangst wrote: On Mon, Dec 30, 2013 at 13:53, Mike Belopuhov wrote: On Mon, Dec 30, 2013 at 03:59 -0800, Loganaden Velvindron wrote: On Mon, Dec 30, 2013 at 12:45:47PM +0100, Mike Belopuhov wrote: On Sun, Dec 29, 2013 at 22:45 -0800, Loganaden Velvindron wrote: Hi All, From NetBSD: Plug memory leak. Coverity CID 1596 memory leak? can you please elaborate where else this memory is leaking if not back to the system. After a short discussion on IRC, and some digging, it makes sense that the system cleanups up automatically, and therefore it is not necessary. the patch can be committed though for the sake of better style. but please review to make sure they are correct. It would be great if the ldconfig, and the various maintainers could review the diff, and help me improve the diffs, or discard them. but the same applies to the ldconfig fix. buildhints is called right before exit. there are no leaks there. the only thing that can be improved is unlinking tmpfilenam. I'm mostly interested in finding the small security issues and fixing them, rather than fixing style issues :-)
user(8) free() before returning in groupmod()
From NetBSD: Coverity annotation -- although memsave free()s its first argument, it will allocate memory and assign it to its first argument, so it is neutral Coverity CID 3228: memory leak -- failed to free() newname in groupmod() Index: src/usr.sbin/user/user.c === RCS file: /cvs/src/usr.sbin/user/user.c,v retrieving revision 1.98 diff -u -p -r1.98 user.c --- src/usr.sbin/user/user.c23 Nov 2013 17:14:05 - 1.98 +++ src/usr.sbin/user/user.c30 Dec 2013 15:58:48 - @@ -2230,7 +2230,8 @@ groupmod(int argc, char **argv) cc = strlcat(buf, \n, sizeof(buf)); if (cc = sizeof(buf)) errx(EXIT_FAILURE, group `%s' entry too long, grp-gr_name); - + if (newname != NULL) + free(newname); openlog(groupmod, LOG_PID, LOG_USER); if (!modify_gid(*argv, buf)) err(EXIT_FAILURE, can't change %s file, _PATH_GROUP);
Re: TOD clock for octeons
On Mon, Dec 30, 2013 at 02:47:58PM +0200, Paul Irofti wrote:
The following enables the DS1337 RTC clock found on octeon boards.
I've tested it on my DSR-500 and would love to hear about tests on other
machines.
Comments? Okays?
New diff after a helpful tip from miod
Index: conf/GENERIC
===
RCS file: /cvs/src/sys/arch/octeon/conf/GENERIC,v
retrieving revision 1.11
diff -u -p -r1.11 GENERIC
--- conf/GENERIC24 Oct 2013 20:47:08 - 1.11
+++ conf/GENERIC30 Dec 2013 16:48:34 -
@@ -31,6 +31,8 @@ clock0at mainbus0
iobus0 at mainbus0
uartbus0 at mainbus0
+octrtc0at mainbus0
+
octcf0 at iobus0
octrng0at iobus0
Index: conf/files.octeon
===
RCS file: /cvs/src/sys/arch/octeon/conf/files.octeon,v
retrieving revision 1.16
diff -u -p -r1.16 files.octeon
--- conf/files.octeon 4 Nov 2013 14:07:16 - 1.16
+++ conf/files.octeon 30 Dec 2013 16:48:34 -
@@ -48,6 +48,11 @@ attach cpu at mainbus
device clock
attach clock at mainbus
+# TOD cloc
+device octrtc
+attach octrtc at mainbus
+file arch/octeon/dev/octrtc.coctrtc
+
define iobus {[base = -1]}
device iobus
attach iobus at mainbus
Index: dev/octrtc.c
===
RCS file: dev/octrtc.c
diff -N dev/octrtc.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ dev/octrtc.c30 Dec 2013 16:48:34 -
@@ -0,0 +1,278 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2013 Paul Irofti.
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include sys/param.h
+#include sys/systm.h
+#include sys/device.h
+#include sys/proc.h
+
+#include mips64/dev/clockvar.h
+
+#include machine/bus.h
+#include machine/autoconf.h
+#include machine/octeonvar.h
+
+#ifdef OCTRTC_DEBUG/* VERY VERBOSE */
+#define DPRINTF(x) printf x
+#else
+#define DPRINTF(x)
+#endif
+
+#define MIO_TWS_SW_TWSI0x000118001000ULL
+#define OCTRTC_REG 0x68
+
+struct octrtc_softc {
+ struct device sc_dev;
+};
+
+struct cfdriver octrtc_cd = {
+ NULL, octrtc, DV_DULL
+};
+
+intoctrtc_match(struct device *, void *, void *);
+void octrtc_attach(struct device *, struct device *, void *);
+
+void octrtc_gettime(void *, time_t, struct tod_time *);
+intoctrtc_read(uint8_t *, char);
+
+void octrtc_settime(void *, struct tod_time *);
+intoctrtc_write(uint8_t);
+
+struct cfattach octrtc_ca = {
+ sizeof(struct octrtc_softc), octrtc_match, octrtc_attach,
+};
+
+
+union mio_tws_sw_twsi_reg {
+ uint64_t reg;
+ struct cvmx_mio_twsx_sw_twsi_s {
+ uint64_t v:1; /* Valid bit */
+ uint64_t slonly:1; /* Slave Only Mode */
+ uint64_t eia:1; /* Extended Internal Address */
+ uint64_t op:4; /* Opcode field */
+ uint64_t r:1; /* Read bit or result */
+ uint64_t sovr:1;/* Size Override */
+ uint64_t size:3;/* Size in bytes */
+ uint64_t scr:2; /* Scratch, unused */
+ uint64_t a:10; /* Address field */
+ uint64_t ia:5; /* Internal Address */
+ uint64_t eop_ia:3; /* Extra opcode */
+ uint64_t d:32; /* Data Field */
+ } field;
+};
+
+
+static inline int bcd2bin(int);
+static inline int
+bcd2bin(int datum)
+{
+ return (datum 4) * 10 + (datum 0x0f);
+}
+static inline int bin2bcd(int);
+static inline int
+bin2bcd(int datum)
+{
+ return ((datum / 10) 4) + (datum % 10);
+}
+
+
+int
+octrtc_match(struct device *parent, void *match, void *aux)
+{
+ return 1;
+}
+
+void
+octrtc_attach(struct device *parent, struct device *self, void *aux)
+{
+ struct octrtc_softc *sc = (struct octrtc_softc *)self;
+
+ sys_tod.tod_cookie = sc;
+ sys_tod.tod_get = octrtc_gettime;
+ sys_tod.tod_set = octrtc_settime;
+
+ printf(: DS1337\n);
+}
+
+void
+octrtc_gettime(void *cookie, time_t unused, struct tod_time *tt)
+{
+ uint8_t tod[8];
+
Re: column memory leak fix
On Mon, Dec 30, 2013 at 04:58:50PM +0100, Mike Belopuhov wrote: On 30 December 2013 16:35, Loganaden Velvindron lo...@elandsys.com wrote: On Mon, Dec 30, 2013 at 08:42:00AM -0500, Ted Unangst wrote: On Mon, Dec 30, 2013 at 13:53, Mike Belopuhov wrote: On Mon, Dec 30, 2013 at 03:59 -0800, Loganaden Velvindron wrote: On Mon, Dec 30, 2013 at 12:45:47PM +0100, Mike Belopuhov wrote: On Sun, Dec 29, 2013 at 22:45 -0800, Loganaden Velvindron wrote: Hi All, From NetBSD: Plug memory leak. Coverity CID 1596 memory leak? can you please elaborate where else this memory is leaking if not back to the system. After a short discussion on IRC, and some digging, it makes sense that the system cleanups up automatically, and therefore it is not necessary. the patch can be committed though for the sake of better style. but please review to make sure they are correct. It would be great if the ldconfig, and the various maintainers could review the diff, and help me improve the diffs, or discard them. but the same applies to the ldconfig fix. buildhints is called right before exit. there are no leaks there. the only thing that can be improved is unlinking tmpfilenam. Maybe so, but ever since I've known of OpenBSD, it has always preached good coding practices and exemplified through its base source code. So why are you discouraging such fixes? You are almost saying that any non-daemon program should not bother with free(), close() and similar resource de-allocation function, because the system will do the reclaiming after program exits. That's rubbish. Regards, --patrick I'm mostly interested in finding the small security issues and fixing them, rather than fixing style issues :-)
Re: column memory leak fix
Loganaden Velvindron lo...@elandsys.com wrote: I'm mostly interested in finding the small security issues and fixing them, rather than fixing style issues :-) Keeping good style helps avoiding bugs, though.
Re: column memory leak fix
On Mon, Dec 30, 2013 at 10:32 PM, patrick keshishian sids...@boxsoft.com wrote: On Mon, Dec 30, 2013 at 04:58:50PM +0100, Mike Belopuhov wrote: On 30 December 2013 16:35, Loganaden Velvindron lo...@elandsys.com wrote: On Mon, Dec 30, 2013 at 08:42:00AM -0500, Ted Unangst wrote: On Mon, Dec 30, 2013 at 13:53, Mike Belopuhov wrote: On Mon, Dec 30, 2013 at 03:59 -0800, Loganaden Velvindron wrote: On Mon, Dec 30, 2013 at 12:45:47PM +0100, Mike Belopuhov wrote: On Sun, Dec 29, 2013 at 22:45 -0800, Loganaden Velvindron wrote: Hi All, From NetBSD: Plug memory leak. Coverity CID 1596 memory leak? can you please elaborate where else this memory is leaking if not back to the system. After a short discussion on IRC, and some digging, it makes sense that the system cleanups up automatically, and therefore it is not necessary. the patch can be committed though for the sake of better style. but please review to make sure they are correct. It would be great if the ldconfig, and the various maintainers could review the diff, and help me improve the diffs, or discard them. but the same applies to the ldconfig fix. buildhints is called right before exit. there are no leaks there. the only thing that can be improved is unlinking tmpfilenam. Maybe so, but ever since I've known of OpenBSD, it has always preached good coding practices and exemplified through its base source code. So why are you discouraging such fixes? You are almost saying that any non-daemon program should not bother with free(), close() and similar resource de-allocation function, because the system will do the reclaiming after program exits. That's rubbish. Hi Patrick, the base source code is pretty huge, and I'm sure that there are memory/fd leak issues lurking around that need to be fixed. I'd much rather spend time finding and fixing them :-) After I got feedback on my diffs, I looked at another base program (user(8)) and asked jj@ to have a look at the diff I just sent. Let the developers decide what they think is the right way. Until they reach consensus, we can use our limited time to fix maximum number of bugs for OpenBSD 5.5 release :-) Regards, --patrick I'm mostly interested in finding the small security issues and fixing them, rather than fixing style issues :-) -- This message is strictly personal and the opinions expressed do not represent those of my employers, either past or present.
Re: user(8) free() before returning in groupmod()
On Mon, Dec 30, 2013 at 08:23, Loganaden Velvindron wrote: Index: src/usr.sbin/user/user.c === RCS file: /cvs/src/usr.sbin/user/user.c,v retrieving revision 1.98 diff -u -p -r1.98 user.c --- src/usr.sbin/user/user.c 23 Nov 2013 17:14:05 - 1.98 +++ src/usr.sbin/user/user.c 30 Dec 2013 15:58:48 - @@ -2230,7 +2230,8 @@ groupmod(int argc, char **argv) cc = strlcat(buf, \n, sizeof(buf)); if (cc = sizeof(buf)) errx(EXIT_FAILURE, group `%s' entry too long, grp-gr_name); - + if (newname != NULL) + free(newname); openlog(groupmod, LOG_PID, LOG_USER); if (!modify_gid(*argv, buf)) err(EXIT_FAILURE, can't change %s file, _PATH_GROUP); This is also poor style. There's no need to check for NULL.
Re: column memory leak fix
On Mon, Dec 30, 2013 at 10:32, patrick keshishian wrote: Maybe so, but ever since I've known of OpenBSD, it has always preached good coding practices and exemplified through its base source code. So why are you discouraging such fixes? You My concern is that these patches are being taken from netbsd without consideration for whether they are the best fix for openbsd, or even correct. That concern is compounded by my sense that the netbsd developers are robotically making changes to appease tools without testing them. I'm a big fan of static analysis and I spent several years working at Coverity, but it is a dangerous tool in the wrong hands.
Add Intel Centrino Wireless-N 2230 support in iwn(4)
Hi tech@,
the diff below adds support for the Intel Centrino Wireless-N 2230
card found in my Lenovo E330 to iwn(4).
iwn0 at pci2 dev 0 function 0 Intel Centrino Wireless-N 2230 rev 0xc4: msi,
MIMO 2T2R, BGN, address 60:6c:66:3b:ea:39
This is the 0x0888 version.
I've got most changes from
https://github.com/seanbruno/freebsd-iwl/commit/53e6056c2df7355650abab77068943ac097a70c6#diff-7a5322b995ac8545b4f5d9096c3b5a5aR5445
which (i think) mostly landed in freebsd as part of
http://svnweb.freebsd.org/base?view=revisionrevision=258035
This is the only iwn(4) device i have, so hopefully i did not broke
another supported device.
Any feedback and tests are welcome.
Regards,
Fabian Raetz
Index: if_iwn.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwn.c,v
retrieving revision 1.127
diff -u -p -r1.127 if_iwn.c
--- if_iwn.c6 Dec 2013 21:03:04 - 1.127
+++ if_iwn.c30 Dec 2013 17:48:59 -
@@ -94,6 +94,8 @@ static const struct pci_matchid iwn_devi
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_130_2 },
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_6235_1 },
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_6235_2 },
+ { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_2x30_1 },
+ { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_2x30_2 },
};
intiwn_match(struct device *, void *, void *);
@@ -244,6 +246,7 @@ int iwn5000_send_calibration(struct iwn
intiwn5000_send_wimax_coex(struct iwn_softc *);
intiwn5000_crystal_calib(struct iwn_softc *);
intiwn5000_temp_offset_calib(struct iwn_softc *);
+intiwn5000_temp_offset_calibv2(struct iwn_softc *);
intiwn4965_post_alive(struct iwn_softc *);
intiwn5000_post_alive(struct iwn_softc *);
intiwn4965_load_bootcode(struct iwn_softc *, const uint8_t *,
@@ -651,6 +654,11 @@ iwn5000_attach(struct iwn_softc *sc, pci
} else
sc-fwname = iwn-6005;
break;
+ case IWN_HW_REV_TYPE_2030:
+ sc-limits = iwn2030_sensitivity_limits;
+ sc-fwname = iwn-2030;
+sc-sc_flags |= IWN_FLAG_ADV_BT_COEX;
+break;
default:
printf(: adapter type %d not supported\n, sc-hw_type);
return ENOTSUP;
@@ -1529,6 +1537,14 @@ iwn5000_read_eeprom(struct iwn_softc *sc
hdr.version, hdr.pa_type, letoh16(hdr.volt)));
sc-calib_ver = hdr.version;
+ if (sc-hw_type == IWN_HW_REV_TYPE_2030) {
+sc-eeprom_voltage = letoh16(hdr.volt);
+iwn_read_prom_data(sc, base + IWN5000_EEPROM_TEMP, val, 2);
+sc-eeprom_temp_high = letoh16(val);
+iwn_read_prom_data(sc, base + IWN5000_EEPROM_VOLT, val, 2);
+sc-eeprom_temp = letoh16(val);
+}
+
if (sc-hw_type == IWN_HW_REV_TYPE_5150) {
/* Compute temperature offset. */
iwn_read_prom_data(sc, base + IWN5000_EEPROM_TEMP, val, 2);
@@ -2095,7 +2111,8 @@ iwn5000_rx_calib_results(struct iwn_soft
switch (calib-code) {
case IWN5000_PHY_CALIB_DC:
- if (sc-hw_type == IWN_HW_REV_TYPE_5150)
+ if (sc-hw_type == IWN_HW_REV_TYPE_5150 ||
+sc-hw_type == IWN_HW_REV_TYPE_2030)
idx = 0;
break;
case IWN5000_PHY_CALIB_LO:
@@ -4161,28 +4178,51 @@ iwn_send_advanced_btcoex(struct iwn_soft
0xc0004000, 0x4000, 0xf0005000, 0xf0005000,
};
struct iwn6000_btcoex_config btconfig;
+struct iwn2000_btcoex_config btconfig2k;
struct iwn_btcoex_priotable btprio;
struct iwn_btcoex_prot btprot;
int error, i;
memset(btconfig, 0, sizeof btconfig);
- btconfig.flags = IWN_BT_FLAG_COEX6000_CHAN_INHIBITION |
- (IWN_BT_FLAG_COEX6000_MODE_3W IWN_BT_FLAG_COEX6000_MODE_SHIFT) |
- IWN_BT_FLAG_SYNC_2_BT_DISABLE;
- btconfig.max_kill = 5;
- btconfig.bt3_t7_timer = 1;
- btconfig.kill_ack = htole32(0x);
- btconfig.kill_cts = htole32(0x);
- btconfig.sample_time = 2;
- btconfig.bt3_t2_timer = 0xc;
- for (i = 0; i 12; i++)
- btconfig.lookup_table[i] = htole32(btcoex_3wire[i]);
- btconfig.valid = htole16(0xff);
- btconfig.prio_boost = 0xf0;
- DPRINTF((configuring advanced bluetooth coexistence\n));
- error = iwn_cmd(sc, IWN_CMD_BT_COEX, btconfig, sizeof(btconfig), 1);
- if (error != 0)
- return (error);
+memset(btconfig2k, 0, sizeof btconfig2k);
+
+if (sc-hw_type == IWN_HW_REV_TYPE_2030) {
+btconfig2k.flags = IWN_BT_FLAG_COEX6000_CHAN_INHIBITION |
+(IWN_BT_FLAG_COEX6000_MODE_3W IWN_BT_FLAG_COEX6000_MODE_SHIFT) |
+IWN_BT_FLAG_SYNC_2_BT_DISABLE;
+btconfig2k.max_kill = 5;
+btconfig2k.bt3_t7_timer = 1;
+btconfig2k.kill_ack =
Re: column memory leak fix
On Mon, Dec 30, 2013 at 22:50, Loganaden Velvindron wrote: Hi Patrick, the base source code is pretty huge, and I'm sure that there are memory/fd leak issues lurking around that need to be fixed. I'd much rather spend time finding and fixing them :-) After I got feedback on my diffs, I looked at another base program (user(8)) and asked jj@ to have a look at the diff I just sent. The resource handling in user is a mess, I'll give you that. It also does some unnecessary freeing, such as immediately before calling errx(). I think I would prefer to see a more comprehensive diff, though, rather than trying to cherry pick changes from netbsd. Read through the source, find the bugs, and think about corrections. Improve the code instead of just tweaking it. (There are plenty of times when small tweaks are better. So far, though, I think we should consider these small bugs as an opportunity for a more comprehensive review of each program.)
Re: column memory leak fix
I'm mostly interested in finding the small security issues and fixing them, rather than fixing style issues :-) As yet, none of these have anything to do with security. Many of the rest are about to get cleaned up by exit, so they are not really fixes of value either. So they come down to style.
Re: TOD clock for octeons
On Mon, Dec 30, 2013 at 06:49:38PM +0200, Paul Irofti wrote:
On Mon, Dec 30, 2013 at 02:47:58PM +0200, Paul Irofti wrote:
The following enables the DS1337 RTC clock found on octeon boards.
I've tested it on my DSR-500 and would love to hear about tests on other
machines.
Comments? Okays?
New diff after a helpful tip from miod
Better diff. There still seems to be problems with setting the time.
I'll look into it in the morning.
Index: conf/GENERIC
===
RCS file: /cvs/src/sys/arch/octeon/conf/GENERIC,v
retrieving revision 1.11
diff -u -p -r1.11 GENERIC
--- conf/GENERIC24 Oct 2013 20:47:08 - 1.11
+++ conf/GENERIC31 Dec 2013 00:21:31 -
@@ -31,6 +31,8 @@ clock0at mainbus0
iobus0 at mainbus0
uartbus0 at mainbus0
+octrtc0at mainbus0
+
octcf0 at iobus0
octrng0at iobus0
Index: conf/files.octeon
===
RCS file: /cvs/src/sys/arch/octeon/conf/files.octeon,v
retrieving revision 1.16
diff -u -p -r1.16 files.octeon
--- conf/files.octeon 4 Nov 2013 14:07:16 - 1.16
+++ conf/files.octeon 31 Dec 2013 00:21:31 -
@@ -48,6 +48,11 @@ attach cpu at mainbus
device clock
attach clock at mainbus
+# TOD clock
+device octrtc
+attach octrtc at mainbus
+file arch/octeon/dev/octrtc.coctrtc
+
define iobus {[base = -1]}
device iobus
attach iobus at mainbus
Index: dev/mainbus.c
===
RCS file: /cvs/src/sys/arch/octeon/dev/mainbus.c,v
retrieving revision 1.5
diff -u -p -r1.5 mainbus.c
--- dev/mainbus.c 25 Jun 2011 19:39:32 - 1.5
+++ dev/mainbus.c 31 Dec 2013 00:21:31 -
@@ -86,6 +86,10 @@ mainbus_attach(struct device *parent, st
/* on-board I/O */
caa.caa_maa.maa_name = iobus;
config_found(self, caa.caa_maa, mainbus_print);
+
+ caa.caa_maa.maa_name = octrtc;
+ config_found(self, caa.caa_maa, mainbus_print);
+
}
int
Index: dev/octrtc.c
===
RCS file: dev/octrtc.c
diff -N dev/octrtc.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ dev/octrtc.c31 Dec 2013 00:21:31 -
@@ -0,0 +1,272 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2013 Paul Irofti.
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include sys/param.h
+#include sys/systm.h
+#include sys/device.h
+#include sys/proc.h
+
+#include mips64/dev/clockvar.h
+
+#include machine/bus.h
+#include machine/autoconf.h
+#include machine/octeonvar.h
+
+#ifdef OCTRTC_DEBUG
+#define DPRINTF(x) printf x
+#else
+#define DPRINTF(x)
+#endif
+
+#define MIO_TWS_SW_TWSI0x000118001000ULL
+#define MIO_TWS_SW_TWSI_EXT0x000118001018ULL
+#define OCTRTC_REG 0x68
+
+struct cfdriver octrtc_cd = {
+ NULL, octrtc, DV_DULL
+};
+
+intoctrtc_match(struct device *, void *, void *);
+void octrtc_attach(struct device *, struct device *, void *);
+
+void octrtc_gettime(void *, time_t, struct tod_time *);
+intoctrtc_read(uint8_t *, char);
+
+void octrtc_settime(void *, struct tod_time *);
+intoctrtc_write(uint8_t);
+
+
+struct cfattach octrtc_ca = {
+ sizeof(struct device), octrtc_match, octrtc_attach,
+};
+
+
+union mio_tws_sw_twsi_reg {
+ uint64_t reg;
+ struct cvmx_mio_twsx_sw_twsi_s {
+ uint64_t v:1; /* Valid bit */
+ uint64_t slonly:1; /* Slave Only Mode */
+ uint64_t eia:1; /* Extended Internal Address */
+ uint64_t op:4; /* Opcode field */
+ uint64_t r:1; /* Read bit or result */
+ uint64_t sovr:1;/* Size Override */
+ uint64_t size:3;/* Size in bytes */
+ uint64_t scr:2; /* Scratch, unused */
+ uint64_t a:10; /* Address field */
+ uint64_t ia:5; /* Internal Address */
+ uint64_t eop_ia:3; /* Extra opcode */
+ uint64_t d:32; /* Data Field */
+ } field;
+};
+
+
+int
Re: base64 b64_pton fix
On Thu, Dec 26, 2013 at 05:17, Todd C. Miller wrote:
This is nit picking but it bugs me to see the conditional repeated
like that. Perhaps instead of:
if (nextbyte tarindex + 1 = targsize)
return (-1);
if (tarindex + 1 targsize)
target[tarindex+1] = nextbyte;
Something more like:
if (tarindex + 1 targsize) {
target[tarindex+1] = nextbyte;
} else if (nextbyte) {
return (-1);
}
Very sensible. I did it that way.
Re: call for testing: MSI for msk(4)
On 16/05/13 5:55 PM, Jérémie Courrèges-Anglas wrote: Hi, I've been using msk(4) with MSI on my laptop since a few days, with no apparent problem. mskc0 at pci2 dev 0 function 0 Marvell Yukon 88E8040 rev 0x13, Yukon-2 FE+ rev. A0 (0x0): msi msk0 at mskc0 port A: address 00:24:54:xx:xx:xx eephy0 at msk0 phy 0: 88E3016 10/100 PHY, rev. 0 Other systems all seem to use MSI, but it would be cool if people with different chips could test it. Any other msk(4) users? It has also been tested with EC 8052/EC Ultra 8058 controllers and working. I don't see any indication of any issues or errata regarding MSI operation in either of the FreeBSD or Linux drivers. -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.
re(4): tedu some unused code
tedu some unused code. it has never been enabled and will not be; to
deal with a hardware defect for rare boards. unmaintained, untested, etc.
want to get rid of it.
Comments? OK?
Index: re.c
===
RCS file: /home/cvs/src/sys/dev/ic/re.c,v
retrieving revision 1.146
diff -u -p -r1.146 re.c
--- re.c25 Oct 2013 22:48:10 - 1.146
+++ re.c31 Dec 2013 00:49:27 -
@@ -197,10 +197,6 @@ void re_setup_intr(struct rl_softc *, in
intre_wol(struct ifnet*, int);
#endif
-#ifdef RE_DIAG
-intre_diag(struct rl_softc *);
-#endif
-
struct cfdriver re_cd = {
0, re, DV_IFNET
};
@@ -615,190 +611,6 @@ re_reset(struct rl_softc *sc)
CSR_WRITE_1(sc, RL_LDPS, 1);
}
-#ifdef RE_DIAG
-
-/*
- * The following routine is designed to test for a defect on some
- * 32-bit 8169 cards. Some of these NICs have the REQ64# and ACK64#
- * lines connected to the bus, however for a 32-bit only card, they
- * should be pulled high. The result of this defect is that the
- * NIC will not work right if you plug it into a 64-bit slot: DMA
- * operations will be done with 64-bit transfers, which will fail
- * because the 64-bit data lines aren't connected.
- *
- * There's no way to work around this (short of talking a soldering
- * iron to the board), however we can detect it. The method we use
- * here is to put the NIC into digital loopback mode, set the receiver
- * to promiscuous mode, and then try to send a frame. We then compare
- * the frame data we sent to what was received. If the data matches,
- * then the NIC is working correctly, otherwise we know the user has
- * a defective NIC which has been mistakenly plugged into a 64-bit PCI
- * slot. In the latter case, there's no way the NIC can work correctly,
- * so we print out a message on the console and abort the device attach.
- */
-
-int
-re_diag(struct rl_softc *sc)
-{
- struct ifnet*ifp = sc-sc_arpcom.ac_if;
- struct mbuf *m0;
- struct ether_header *eh;
- struct rl_rxsoft*rxs;
- struct rl_desc *cur_rx;
- bus_dmamap_tdmamap;
- u_int16_t status;
- u_int32_t rxstat;
- int total_len, i, s, error = 0, phyaddr;
- u_int8_tdst[] = { 0x00, 'h', 'e', 'l', 'l', 'o' };
- u_int8_tsrc[] = { 0x00, 'w', 'o', 'r', 'l', 'd' };
-
- DPRINTF((inside re_diag\n));
- /* Allocate a single mbuf */
-
- MGETHDR(m0, M_DONTWAIT, MT_DATA);
- if (m0 == NULL)
- return (ENOBUFS);
-
- /*
-* Initialize the NIC in test mode. This sets the chip up
-* so that it can send and receive frames, but performs the
-* following special functions:
-* - Puts receiver in promiscuous mode
-* - Enables digital loopback mode
-* - Leaves interrupts turned off
-*/
-
- ifp-if_flags |= IFF_PROMISC;
- sc-rl_testmode = 1;
- re_reset(sc);
- re_init(ifp);
- sc-rl_flags |= RL_FLAG_LINK;
- if (sc-sc_hwrev == RL_HWREV_8139CPLUS)
- phyaddr = 0;
- else
- phyaddr = 1;
-
- re_miibus_writereg((struct device *)sc, phyaddr, MII_BMCR,
- BMCR_RESET);
- for (i = 0; i RL_TIMEOUT; i++) {
- status = re_miibus_readreg((struct device *)sc,
- phyaddr, MII_BMCR);
- if (!(status BMCR_RESET))
- break;
- }
-
- re_miibus_writereg((struct device *)sc, phyaddr, MII_BMCR,
- BMCR_LOOP);
- CSR_WRITE_2(sc, RL_ISR, RL_INTRS);
-
- DELAY(10);
-
- /* Put some data in the mbuf */
-
- eh = mtod(m0, struct ether_header *);
- bcopy ((char *)dst, eh-ether_dhost, ETHER_ADDR_LEN);
- bcopy ((char *)src, eh-ether_shost, ETHER_ADDR_LEN);
- eh-ether_type = htons(ETHERTYPE_IP);
- m0-m_pkthdr.len = m0-m_len = ETHER_MIN_LEN - ETHER_CRC_LEN;
-
- /*
-* Queue the packet, start transmission.
-*/
-
- CSR_WRITE_2(sc, RL_ISR, 0x);
- s = splnet();
- IFQ_ENQUEUE(ifp-if_snd, m0, NULL, error);
- re_start(ifp);
- splx(s);
- m0 = NULL;
-
- DPRINTF((re_diag: transmission started\n));
-
- /* Wait for it to propagate through the chip */
-
- DELAY(10);
- for (i = 0; i RL_TIMEOUT; i++) {
- status = CSR_READ_2(sc, RL_ISR);
- CSR_WRITE_2(sc, RL_ISR, status);
- if ((status (RL_ISR_TIMEOUT_EXPIRED|RL_ISR_RX_OK)) ==
- (RL_ISR_TIMEOUT_EXPIRED|RL_ISR_RX_OK))
- break;
- DELAY(10);
- }
- if (i == RL_TIMEOUT) {
- printf(%s: diagnostic failed, failed to receive packet
- in loopback mode\n, sc-sc_dev.dv_xname);
- error =
em(4): Don't count RX overruns and missed packets as input errros
Don't count RX overruns and missed packets as inputs errors. They're expected to increment when using MCLGETI. OK? Index: if_em.c === RCS file: /cvs/src/sys/dev/pci/if_em.c,v retrieving revision 1.275 diff -u -p -u -p -r1.275 if_em.c --- if_em.c 28 Dec 2013 03:34:54 - 1.275 +++ if_em.c 31 Dec 2013 06:03:55 - @@ -3246,9 +3248,9 @@ em_update_stats_counters(struct em_softc sc-stats.rxerrc + sc-stats.crcerrs + sc-stats.algnerrc + - sc-stats.ruc + sc-stats.roc + - sc-stats.mpc + sc-stats.cexterr + - sc-rx_overruns; + sc-stats.ruc + + sc-stats.roc + + sc-stats.cexterr; /* Tx Errors */ ifp-if_oerrors = sc-stats.ecol + sc-stats.latecol + -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.
