libssl buffer.c change memset before free to explicit_bzero
- change memset before free to explicit_bzero - change ordinary memset's to bzero - change if(!data) malloc else realloc to realloc - explicit_bzero on downsize instead of memset Possible an advantage (the only?) of the previous allocator was this memset would have never been optimised out. Sorry for the git patch diff --git a/lib/libssl/src/crypto/buffer/buffer.c b/lib/libssl/src/crypto/buffer/buffer.c index 486d6fe..fde718f 100644 --- a/lib/libssl/src/crypto/buffer/buffer.c +++ b/lib/libssl/src/crypto/buffer/buffer.c @@ -88,7 +88,7 @@ BUF_MEM_free(BUF_MEM *a) return; if (a-data != NULL) { - memset(a-data, 0, (unsigned int)a-max); + explicit_bzero(a-data, a-max); free(a-data); } free(a); @@ -105,7 +105,7 @@ BUF_MEM_grow(BUF_MEM *str, size_t len) return (len); } if (str-max = len) { - memset(str-data[str-length], 0, len - str-length); + bzero(str-data[str-length], len - str-length); str-length = len; return (len); } @@ -115,17 +115,15 @@ BUF_MEM_grow(BUF_MEM *str, size_t len) return 0; } n = (len + 3) / 3 * 4; - if (str-data == NULL) - ret = malloc(n); - else - ret = realloc(str-data, n); + ret = realloc(str-data, n); + if (ret == NULL) { BUFerr(BUF_F_BUF_MEM_GROW, ERR_R_MALLOC_FAILURE); len = 0; } else { str-data = ret; str-max = n; - memset(str-data[str-length], 0, len - str-length); + bzero(str-data[str-length], len - str-length); str-length = len; } return (len); @@ -138,12 +136,12 @@ BUF_MEM_grow_clean(BUF_MEM *str, size_t len) size_t n; if (str-length = len) { - memset(str-data[len], 0, str-length - len); + explicit_bzero(str-data[len], str-length - len); str-length = len; return (len); } if (str-max = len) { - memset(str-data[str-length], 0, len - str-length); + bzero(str-data[str-length], len - str-length); str-length = len; return (len); } @@ -166,7 +164,7 @@ BUF_MEM_grow_clean(BUF_MEM *str, size_t len) } else { str-data = ret; str-max = n; - memset(str-data[str-length], 0, len - str-length); + bzero(str-data[str-length], len - str-length); str-length = len; } return (len);
Re: 9p
On Fri, May 30, 2014 at 10:42 PM, M Farkas-Dyck strake...@gmail.com wrote: Ls seems to stat the directory and allocate a large enough dent buffer. I couldn't find what ls calls to do so, but I assume that it's a common function and other programs use it too. opendir() and readdir() are the functions you're looking for. They're the ones that open a directory and call getdents() to read the entries from it. They're both standardized and used *everywhere* for reading directories, so if you want this file system to work at all, it must work for those functions. Note that getdents() is used by some ports, so it must work correctly too; you can't just hack around problems by changing readdir(). ls invokes those via fts_open() and related functions. It's used by basically all BSD programs that might need to traverse a directory tree. Problem is that directories in 9p customarily have length 0, so ls says every such directory empty. Getdents itself works. It returns dirent structures, as defined in sys/dirent.h, with non-zero d_fileno, with d_off values that are seek offsets to the succeeding entry in the directory, with d_reclen of the padded and 8-byte aligned size of the directory entry, with d_type of either DT_UNKNOWN or the appropriate DT_* value, with d_name of the desired NUL-terminated name, and with d_namlen the length of the name? 3 options immediately come to mind: 1. Modify said function to not care how long stat says the directory is, choose an arbitrary dent buffer size, and iterate if need be 2. Read full directory on every stat call, blah 3. Lie that every directory length is (off_t)(-1) and hope that programs know to not try to allocate so much... opendir/readdir don't look at the st_size member, so I don't think any of those will help. You haven't found the real problem yet. Philip Guenther
[PATCH] libssl: malloc checks for NULL
Hi tech@ Here are some missing checks for NULL after malloc. I sent these in a bit ago, but I didn't see them picked up. Regards Index: src/apps/apps.c === RCS file: /cvs/src/lib/libssl/src/apps/apps.c,v retrieving revision 1.56 diff -u -p -r1.56 apps.c --- src/apps/apps.c 30 May 2014 04:59:14 - 1.56 +++ src/apps/apps.c 31 May 2014 06:47:13 - @@ -215,6 +215,8 @@ chopup_args(ARGS *arg, char *buf, int *a if (arg-count == 0) { arg-count = 20; arg-data = reallocarray(NULL, arg-count, sizeof(char *)); + if (arg-data == NULL) + return (0); } for (i = 0; i arg-count; i++) arg-data[i] = NULL; Index: src/apps/dgst.c === RCS file: /cvs/src/lib/libssl/src/apps/dgst.c,v retrieving revision 1.34 diff -u -p -r1.34 dgst.c --- src/apps/dgst.c 23 May 2014 16:10:02 - 1.34 +++ src/apps/dgst.c 31 May 2014 06:47:13 - @@ -401,6 +401,11 @@ mac_end: sigbio = BIO_new_file(sigfile, rb); siglen = EVP_PKEY_size(sigkey); sigbuf = malloc(siglen); + if (sigbuf == NULL) { + BIO_printf(bio_err, Out of memory\n); + ERR_print_errors(bio_err); + goto end; + } if (!sigbio) { BIO_printf(bio_err, Error opening signature file %s\n, sigfile); Index: src/apps/speed.c === RCS file: /cvs/src/lib/libssl/src/apps/speed.c,v retrieving revision 1.45 diff -u -p -r1.45 speed.c --- src/apps/speed.c29 May 2014 21:07:42 - 1.45 +++ src/apps/speed.c31 May 2014 06:47:13 - @@ -2107,6 +2107,10 @@ do_multi(int multi) static char sep[] = :; fds = reallocarray(NULL, multi, sizeof *fds); + if (fds == NULL) { + fprintf(stderr, Out of memory\n); + exit(1); + } for (n = 0; n multi; ++n) { if (pipe(fd) == -1) { fprintf(stderr, pipe failure\n); Index: src/apps/x509.c === RCS file: /cvs/src/lib/libssl/src/apps/x509.c,v retrieving revision 1.44 diff -u -p -r1.44 x509.c --- src/apps/x509.c 23 May 2014 16:10:02 - 1.44 +++ src/apps/x509.c 31 May 2014 06:47:13 - @@ -746,6 +746,11 @@ bad: z = i2d_X509(x, NULL); m = malloc(z); + if (m == NULL) { + BIO_printf(bio_err, Out of memory\n); + ERR_print_errors(bio_err); + goto end; + } d = (unsigned char *) m; z = i2d_X509_NAME(X509_get_subject_name(x), d); Index: src/crypto/asn1/bio_ndef.c === RCS file: /cvs/src/lib/libssl/src/crypto/asn1/bio_ndef.c,v retrieving revision 1.7 diff -u -p -r1.7 bio_ndef.c --- src/crypto/asn1/bio_ndef.c 30 May 2014 02:52:11 - 1.7 +++ src/crypto/asn1/bio_ndef.c 31 May 2014 06:47:14 - @@ -164,6 +164,8 @@ ndef_prefix(BIO *b, unsigned char **pbuf derlen = ASN1_item_ndef_i2d(ndef_aux-val, NULL, ndef_aux-it); p = malloc(derlen); + if (p == NULL) + return (0); ndef_aux-derbuf = p; *pbuf = p; derlen = ASN1_item_ndef_i2d(ndef_aux-val, p, ndef_aux-it); @@ -231,6 +233,8 @@ ndef_suffix(BIO *b, unsigned char **pbuf derlen = ASN1_item_ndef_i2d(ndef_aux-val, NULL, ndef_aux-it); p = malloc(derlen); + if (p == NULL) + return (0); ndef_aux-derbuf = p; *pbuf = p; derlen = ASN1_item_ndef_i2d(ndef_aux-val, p, ndef_aux-it); Index: src/crypto/cms/cms_pwri.c === RCS file: /cvs/src/lib/libssl/src/crypto/cms/cms_pwri.c,v retrieving revision 1.4 diff -u -p -r1.4 cms_pwri.c --- src/crypto/cms/cms_pwri.c 24 May 2014 15:55:21 - 1.4 +++ src/crypto/cms/cms_pwri.c 31 May 2014 06:47:18 - @@ -231,6 +231,8 @@ kek_unwrap_key(unsigned char *out, size_ return 0; } tmp = malloc(inlen); + if (tmp == NULL) + return (0); /* setup IV by decrypting last two blocks */ EVP_DecryptUpdate(ctx, tmp + inlen - 2 * blocklen, outl, in + inlen - 2 * blocklen, blocklen * 2);
Re: [PATCH]unnecessary return in arc4random
Am I wrong? On Thu, May 22, 2014 at 04:30:03PM +0200, Fritjof Bornebusch wrote: Hi tech, does this return makes any sense, because it's a void function and the return is at the end of the function. fritjof Index: arc4random.c === RCS file: /cvs/src/lib/libc/crypt/arc4random.c,v retrieving revision 1.30 diff -u -p -r1.30 arc4random.c --- arc4random.c6 May 2014 16:06:33 - 1.30 +++ arc4random.c22 May 2014 14:21:35 - @@ -165,7 +165,6 @@ _rs_random_u32(u_int32_t *val) memcpy(val, rs_buf + RSBUFSZ - rs_have, sizeof(*val)); memset(rs_buf + RSBUFSZ - rs_have, 0, sizeof(*val)); rs_have -= sizeof(*val); - return; } u_int32_t
Re: libssl buffer.c change memset before free to explicit_bzero
On Sat, May 31, 2014 at 17:00, Cameron Stewart wrote: - change memset before free to explicit_bzero - change ordinary memset's to bzero Actually, memset is preferred to bzero in the ordinary case. Another good reason to make separate diffs for separate changes. :)
Re: [PATCH]unnecessary return in arc4random
Fritjof Bornebusch frit...@alokat.org writes: Am I wrong? Nope. Committed, thanks. On Thu, May 22, 2014 at 04:30:03PM +0200, Fritjof Bornebusch wrote: Hi tech, does this return makes any sense, because it's a void function and the return is at the end of the function. fritjof Index: arc4random.c === RCS file: /cvs/src/lib/libc/crypt/arc4random.c,v retrieving revision 1.30 diff -u -p -r1.30 arc4random.c --- arc4random.c6 May 2014 16:06:33 - 1.30 +++ arc4random.c22 May 2014 14:21:35 - @@ -165,7 +165,6 @@ _rs_random_u32(u_int32_t *val) memcpy(val, rs_buf + RSBUFSZ - rs_have, sizeof(*val)); memset(rs_buf + RSBUFSZ - rs_have, 0, sizeof(*val)); rs_have -= sizeof(*val); - return; } u_int32_t -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
[PATCH 8] aic7xxx: malloc/memset = calloc
Index: aicasm.c === RCS file: /cvs/src/sys/dev/microcode/aic7xxx/aicasm.c,v retrieving revision 1.15 diff -u -p -r1.15 aicasm.c --- aicasm.c5 Dec 2012 23:20:19 - 1.15 +++ aicasm.c31 May 2014 11:01:21 - @@ -500,12 +500,10 @@ emit_patch(scope_t *scope, int patch) /* No-Op patch */ return; - new_patch = (patch_t *)malloc(sizeof(*new_patch)); + new_patch = calloc(1, sizeof(*new_patch)); if (new_patch == NULL) - stop(Could not malloc patch structure, EX_OSERR); - - memset(new_patch, 0, sizeof(*new_patch)); + stop(Could not calloc patch structure, EX_OSERR); if (patch == 0) { new_patch-patch_func = scope-func_num; @@ -737,10 +735,9 @@ seq_alloc() { struct instruction *new_instr; - new_instr = (struct instruction *)malloc(sizeof(struct instruction)); + new_instr = calloc(1, sizeof(struct instruction)); if (new_instr == NULL) - stop(Unable to malloc instruction object, EX_SOFTWARE); - memset(new_instr, 0, sizeof(*new_instr)); + stop(Unable to calloc instruction object, EX_SOFTWARE); TAILQ_INSERT_TAIL(seq_program, new_instr, links); new_instr-srcline = yylineno; return new_instr; @@ -751,10 +748,9 @@ cs_alloc() { critical_section_t *new_cs; - new_cs= (critical_section_t *)malloc(sizeof(critical_section_t)); + new_cs = calloc(1, sizeof(critical_section_t)); if (new_cs == NULL) - stop(Unable to malloc critical_section object, EX_SOFTWARE); - memset(new_cs, 0, sizeof(*new_cs)); + stop(Unable to calloc critical_section object, EX_SOFTWARE); TAILQ_INSERT_TAIL(cs_tailq, new_cs, links); return new_cs; @@ -765,10 +761,9 @@ scope_alloc() { scope_t *new_scope; - new_scope = (scope_t *)malloc(sizeof(scope_t)); + new_scope = calloc(1, sizeof(scope_t)); if (new_scope == NULL) - stop(Unable to malloc scope object, EX_SOFTWARE); - memset(new_scope, 0, sizeof(*new_scope)); + stop(Unable to calloc scope object, EX_SOFTWARE); TAILQ_INIT(new_scope-inner_scope); if (SLIST_FIRST(scope_stack) != NULL) { Index: aicasm_symbol.c === RCS file: /cvs/src/sys/dev/microcode/aic7xxx/aicasm_symbol.c,v retrieving revision 1.11 diff -u -p -r1.11 aicasm_symbol.c --- aicasm_symbol.c 5 Dec 2012 23:20:19 - 1.11 +++ aicasm_symbol.c 31 May 2014 11:01:21 - @@ -68,12 +68,11 @@ symbol_create(char *name) { symbol_t *new_symbol; - new_symbol = (symbol_t *)malloc(sizeof(symbol_t)); + new_symbol = calloc(1, sizeof(symbol_t)); if (new_symbol == NULL) { perror(Unable to create new symbol); exit(EX_SOFTWARE); } - memset(new_symbol, 0, sizeof(*new_symbol)); new_symbol-name = strdup(name); if (new_symbol-name == NULL) stop(Unable to strdup symbol name, EX_SOFTWARE);
[PATCH 9] installboot: malloc/memset = calloc
This one splits up the malloc parameter, taking full potential from calloc, hurting readability a bit. which one is preferred? more readable/maintainable or using the calloc overflow protection? Index: bootstrap.c === RCS file: /cvs/src/usr.sbin/installboot/bootstrap.c,v retrieving revision 1.3 diff -u -p -r1.3 bootstrap.c --- bootstrap.c 28 Dec 2013 12:01:33 - 1.3 +++ bootstrap.c 31 May 2014 10:26:27 - @@ -68,10 +68,9 @@ bootstrap(int devfd, char *dev, char *bo fprintf(stderr, bootstrap is %zu bytes (%zu sectors @ %u bytes = %zu bytes)\n, (ssize_t)sb.st_size, bootsec, dl.d_secsize, bootsize); - boot = malloc(bootsize); + boot = calloc(bootsec, dl.d_secsize); if (boot == NULL) - err(1, malloc); - memset(boot, 0, bootsize); + err(1, calloc); if (read(fd, boot, bootsize) != (ssize_t)sb.st_size) err(1, read); close(fd); Index: i386_softraid.c === RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v retrieving revision 1.1 diff -u -p -r1.1 i386_softraid.c --- i386_softraid.c 19 Jan 2014 02:58:50 - 1.1 +++ i386_softraid.c 31 May 2014 10:26:27 - @@ -129,11 +129,10 @@ sr_install_bootldr(int devfd, char *dev) inodeblk = nblocks - 1; bootsize = nblocks * SR_FS_BLOCKSIZE; - p = malloc(bootsize); + p = calloc(nblocks, SR_FS_BLOCKSIZE); if (p == NULL) err(1, NULL); - memset(p, 0, bootsize); fd = open(stage2, O_RDONLY, 0); if (fd == -1) err(1, NULL); Index: sparc64_installboot.c === RCS file: /cvs/src/usr.sbin/installboot/sparc64_installboot.c,v retrieving revision 1.1 diff -u -p -r1.1 sparc64_installboot.c --- sparc64_installboot.c 19 Jan 2014 02:58:50 - 1.1 +++ sparc64_installboot.c 31 May 2014 10:26:27 - @@ -68,10 +68,9 @@ md_loadboot(void) if (blksize SBSIZE - DEV_BSIZE) errx(1, boot blocks too big (%zu %d), blksize, SBSIZE - DEV_BSIZE); - blkstore = malloc(blksize); + blkstore = calloc(blocks, DEV_BSIZE); if (blkstore == NULL) - err(1, malloc); - memset(blkstore, 0, blksize); + err(1, calloc); if (read(fd, blkstore, sb.st_size) != (ssize_t)sb.st_size) err(1, read); close(fd);
[PATCH 10] inetd: malloc/memset = calloc
While here also stop casting {m,c}alloc return value. Index: inetd.c === RCS file: /cvs/src/usr.sbin/inetd/inetd.c,v retrieving revision 1.137 diff -u -p -r1.137 inetd.c --- inetd.c 23 Nov 2013 17:24:29 - 1.137 +++ inetd.c 31 May 2014 13:58:00 - @@ -455,7 +455,7 @@ main(int argc, char *argv[]) if (readablen != allsockn) { if (fdsrp) free(fdsrp); - fdsrp = (fd_set *)calloc(allsockn, 1); + fdsrp = calloc(allsockn, 1); if (fdsrp == NULL) { syslog(LOG_ERR, Out of memory.); exit(1); @@ -1085,7 +1085,7 @@ enter(struct servtab *cp) struct servtab *sep; sigset_t omask; - sep = (struct servtab *)malloc(sizeof (*sep)); + sep = malloc(sizeof (*sep)); if (sep == NULL) { syslog(LOG_ERR, Out of memory.); exit(1); @@ -1180,13 +1180,11 @@ getconfigent(void) char *arg, *cp, *hostdelim, *s; int argc; - sep = (struct servtab *) malloc(sizeof(struct servtab)); + sep = calloc(1, sizeof(struct servtab)); if (sep == NULL) { - syslog(LOG_ERR, malloc: %m); + syslog(LOG_ERR, calloc: %m); exit(1); } - - memset(sep, 0, sizeof *sep); more: freeconfig(sep); @@ -1512,14 +1510,12 @@ dupconfig(struct servtab *sep) struct servtab *newtab; int argc; - newtab = (struct servtab *) malloc(sizeof(struct servtab)); + newtab = calloc(1, sizeof(struct servtab)); if (newtab == NULL) { - syslog(LOG_ERR, malloc: %m); + syslog(LOG_ERR, calloc: %m); exit(1); } - - memset(newtab, 0, sizeof(struct servtab)); newtab-se_service = sep-se_service ? newstr(sep-se_service) : NULL; newtab-se_socktype = sep-se_socktype;
Re: [PATCH 9] installboot: malloc/memset = calloc
On Sat, 31 May 2014, Benjamin Baier wrote: This one splits up the malloc parameter, taking full potential from calloc, hurting readability a bit. which one is preferred? more readable/maintainable or using the calloc overflow protection? In this case I think readability wins. I do not believe that there is a lot to gain from overflow protection given the numbers used in these calculations are very small (and many are already bounds checked in some form or other). Index: bootstrap.c === RCS file: /cvs/src/usr.sbin/installboot/bootstrap.c,v retrieving revision 1.3 diff -u -p -r1.3 bootstrap.c --- bootstrap.c 28 Dec 2013 12:01:33 - 1.3 +++ bootstrap.c 31 May 2014 10:26:27 - @@ -68,10 +68,9 @@ bootstrap(int devfd, char *dev, char *bo fprintf(stderr, bootstrap is %zu bytes (%zu sectors @ %u bytes = %zu bytes)\n, (ssize_t)sb.st_size, bootsec, dl.d_secsize, bootsize); - boot = malloc(bootsize); + boot = calloc(bootsec, dl.d_secsize); if (boot == NULL) - err(1, malloc); - memset(boot, 0, bootsize); + err(1, calloc); if (read(fd, boot, bootsize) != (ssize_t)sb.st_size) err(1, read); close(fd); Index: i386_softraid.c === RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v retrieving revision 1.1 diff -u -p -r1.1 i386_softraid.c --- i386_softraid.c 19 Jan 2014 02:58:50 - 1.1 +++ i386_softraid.c 31 May 2014 10:26:27 - @@ -129,11 +129,10 @@ sr_install_bootldr(int devfd, char *dev) inodeblk = nblocks - 1; bootsize = nblocks * SR_FS_BLOCKSIZE; - p = malloc(bootsize); + p = calloc(nblocks, SR_FS_BLOCKSIZE); if (p == NULL) err(1, NULL); - memset(p, 0, bootsize); fd = open(stage2, O_RDONLY, 0); if (fd == -1) err(1, NULL); Index: sparc64_installboot.c === RCS file: /cvs/src/usr.sbin/installboot/sparc64_installboot.c,v retrieving revision 1.1 diff -u -p -r1.1 sparc64_installboot.c --- sparc64_installboot.c 19 Jan 2014 02:58:50 - 1.1 +++ sparc64_installboot.c 31 May 2014 10:26:27 - @@ -68,10 +68,9 @@ md_loadboot(void) if (blksize SBSIZE - DEV_BSIZE) errx(1, boot blocks too big (%zu %d), blksize, SBSIZE - DEV_BSIZE); - blkstore = malloc(blksize); + blkstore = calloc(blocks, DEV_BSIZE); if (blkstore == NULL) - err(1, malloc); - memset(blkstore, 0, blksize); + err(1, calloc); if (read(fd, blkstore, sb.st_size) != (ssize_t)sb.st_size) err(1, read); close(fd); -- Action without study is fatal. Study without action is futile. -- Mary Ritter Beard
LibreSSL and ASN.1 encodings
a short plea from someone who just had to dig through OpenSSL code and figuring out why I was getting T.61 strings instead of UTF8Strings. Would you _please_ and kindly change the default of global_mask in crypto/asn1/a_strnid.c from 0xL to B_ASN1_UTF8STRING OpenSSL upstream has had the mask set to utf8only in the config file since 2005, but kept the default to the code. This makes OpenSSL by default non-compatible with RFC5280, and is generally a headache to work with. Kind regards, D.S. - who just spent hours chasing that behaviour down in OpenSSL.
Remove unneeded patch to alias 646 = ascii in Encode::Alias
Turns out in 2008 the regex that aliases ISO-646 to ascii was changed so that it also matches plain 646, so we no longer need a patch to do that. https://github.com/dankogai/p5-encode/commit/aa1fdef85add72114ed7badf85e523cfdc9d6275 They adjusted the regex like this, making the leading ISO optional: -define_alias( qr/\bISO[-_]?646[-_]?US$/i = 'ascii' ); +define_alias( qr/\b(?:ISO[-_]?)?646(?:[-_]?US)?$/i = 'ascii' ); The secondary quotes on the definition ('ascii') are due to the way regex aliasing works and get stripped off. OK to remove our local patch? -- andrew - http://afresh1.com ($do || !$do) undef($try) ; # Master of Perl, Yoda is. H? Index: gnu/usr.bin/perl/cpan/Encode/lib/Encode/Alias.pm === RCS file: /cvs/src/gnu/usr.bin/perl/cpan/Encode/lib/Encode/Alias.pm,v retrieving revision 1.4 diff -u -p -u -r1.4 Alias.pm --- gnu/usr.bin/perl/cpan/Encode/lib/Encode/Alias.pm24 Mar 2014 15:05:24 - 1.4 +++ gnu/usr.bin/perl/cpan/Encode/lib/Encode/Alias.pm31 May 2014 17:08:19 - @@ -152,7 +152,6 @@ sub init_aliases { define_alias( qr/^(?:US-?)ascii$/i = 'ascii' ); define_alias( 'C'= 'ascii' ); define_alias( qr/\b(?:ISO[-_]?)?646(?:[-_]?US)?$/i = 'ascii' ); -define_alias( '646' = 'ascii' ); # Allow variants of iso-8859-1 etc. define_alias( qr/\biso[-_]?(\d+)[-_](\d+)$/i = 'iso-$1-$2' );
Wifi TL W723N
Hi, I finally bought a TL W723N and try. usbdevs gives: port 2 addr 3: high speed, power 500 mA, config 1, 802.11n NIC(0x8179), Realtek(0x0bda), rev 0.00, iSerialNumber 00E04C0001 0x8179 is not present in kernel source (usbdevs.h) (for realteak there is 0x8178 and 0x817a and around but not this one). It is not detected (I'm using -current): ugen1 at uhub3 port 2 Realtek 802.11n NIC rev 2.00/0.00 addr 3 I try to modify usbdevs.h and rsu drivers with no result: rsu0: MAC/BB RTL8712 cut 3, address 1a:00:00:00:00:00 rsu0: could not open bulk pipe 0x83 Then I try to modify urtwn drivers: urtwn0 at uhub3 port 2 Realtek 802.11n NIC rev 2.00/0.00 addr 3 urtwn0: MAC/BB RTL8188CUS, RF 6052 1T1R, address 00:00:30:34:43:30 # ifconfig urtwn0 nwid LSA wpapsk ** # ifconfig urtwn0 urtwn0: flags=8803UP,BROADCAST,SIMPLEX,MULTICAST mtu 1500 lladdr 00:00:30:34:43:30 priority: 4 groups: wlan media: IEEE802.11 autoselect status: no network ieee80211: nwid LSA wpakey not displayed wpaprotos wpa1,wpa2 wpaakms psk wpaciphers tkip,ccmp wpagroupcipher tkip inet6 fe80::200:30ff:fe34:4330%urtwn0 prefixlen 64 scopeid 0x6 # dhclient urtwn0 urtwn0: no link . sleeping any idea how to get it works (if possible of course)? Regards, Sébastien
Re: [PATCH 9] installboot: malloc/memset = calloc
On Sun, 1 Jun 2014 00:57:43 +1000 Joel Sing j...@sing.id.au wrote: In this case I think readability wins. I do not believe that there is a lot to gain from overflow protection given the numbers used in these calculations are very small (and many are already bounds checked in some form or other). Well, I had to ask. Here is option 2. Index: bootstrap.c === RCS file: /cvs/src/usr.sbin/installboot/bootstrap.c,v retrieving revision 1.3 diff -u -p -r1.3 bootstrap.c --- bootstrap.c 28 Dec 2013 12:01:33 - 1.3 +++ bootstrap.c 31 May 2014 18:14:56 - @@ -68,10 +68,9 @@ bootstrap(int devfd, char *dev, char *bo fprintf(stderr, bootstrap is %zu bytes (%zu sectors @ %u bytes = %zu bytes)\n, (ssize_t)sb.st_size, bootsec, dl.d_secsize, bootsize); - boot = malloc(bootsize); + boot = calloc(1, bootsize); if (boot == NULL) - err(1, malloc); - memset(boot, 0, bootsize); + err(1, calloc); if (read(fd, boot, bootsize) != (ssize_t)sb.st_size) err(1, read); close(fd); Index: i386_softraid.c === RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v retrieving revision 1.1 diff -u -p -r1.1 i386_softraid.c --- i386_softraid.c 19 Jan 2014 02:58:50 - 1.1 +++ i386_softraid.c 31 May 2014 18:14:56 - @@ -129,11 +129,10 @@ sr_install_bootldr(int devfd, char *dev) inodeblk = nblocks - 1; bootsize = nblocks * SR_FS_BLOCKSIZE; - p = malloc(bootsize); + p = calloc(1, bootsize); if (p == NULL) err(1, NULL); - memset(p, 0, bootsize); fd = open(stage2, O_RDONLY, 0); if (fd == -1) err(1, NULL); Index: sparc64_installboot.c === RCS file: /cvs/src/usr.sbin/installboot/sparc64_installboot.c,v retrieving revision 1.1 diff -u -p -r1.1 sparc64_installboot.c --- sparc64_installboot.c 19 Jan 2014 02:58:50 - 1.1 +++ sparc64_installboot.c 31 May 2014 18:14:56 - @@ -68,10 +68,9 @@ md_loadboot(void) if (blksize SBSIZE - DEV_BSIZE) errx(1, boot blocks too big (%zu %d), blksize, SBSIZE - DEV_BSIZE); - blkstore = malloc(blksize); + blkstore = calloc(1, blksize); if (blkstore == NULL) - err(1, malloc); - memset(blkstore, 0, blksize); + err(1, calloc); if (read(fd, blkstore, sb.st_size) != (ssize_t)sb.st_size) err(1, read); close(fd);
Re: LibreSSL and ASN.1 encodings
Done. Thanks for the giggle. I needed it today. -Bob On Sat, May 31, 2014 at 07:20:56PM +0200, D. Spindel wrote: a short plea from someone who just had to dig through OpenSSL code and figuring out why I was getting T.61 strings instead of UTF8Strings. Would you _please_ and kindly change the default of global_mask in crypto/asn1/a_strnid.c from 0xL to B_ASN1_UTF8STRING OpenSSL upstream has had the mask set to utf8only in the config file since 2005, but kept the default to the code. This makes OpenSSL by default non-compatible with RFC5280, and is generally a headache to work with. Kind regards, D.S. - who just spent hours chasing that behaviour down in OpenSSL.
Re: LibreSSL and ASN.1 encodings
Some historical anecdotes: T.61 was proposed in 93. Utf8 later the same year. utf8 was recommended from 94. 2004 OpenSSL caught up with the recommendation, and decided to go against it to be compatible with Netscape Navigator. Which at that time had a massive 2% of the market. 2005 The behaviour of the openssl binaries were fixed by changing the config file. 2014 the default still hasn't been changed, 20 years after the original deprecation of T.61 in x509 standards. I love the speed with which this evolves. Thanks for fixing this, it's very appreciated, even if I have to keep hacking around the behaviour in openssl for the next decade as well. //D.S. On Sat, May 31, 2014 at 10:17 PM, Bob Beck b...@openbsd.org wrote: Done. Thanks for the giggle. I needed it today. -Bob On Sat, May 31, 2014 at 07:20:56PM +0200, D. Spindel wrote: a short plea from someone who just had to dig through OpenSSL code and figuring out why I was getting T.61 strings instead of UTF8Strings. Would you _please_ and kindly change the default of global_mask in crypto/asn1/a_strnid.c from 0xL to B_ASN1_UTF8STRING OpenSSL upstream has had the mask set to utf8only in the config file since 2005, but kept the default to the code. This makes OpenSSL by default non-compatible with RFC5280, and is generally a headache to work with. Kind regards, D.S. - who just spent hours chasing that behaviour down in OpenSSL.
Re: 9p
On 31/05/2014, Philip Guenther guent...@gmail.com wrote: opendir/readdir don't look at the st_size member, so I don't think any of those will help. You haven't found the real problem yet. Yes, I missed this in getdents docs: nbytes must be greater than or equal to the block size associated with the file. With block size great enough to hold a full 9p stat structure, it now works. Thanks for the help.
Re: Remove unneeded patch to alias 646 = ascii in Encode::Alias
On Sat, May 31, 2014 at 10:22 AM, Andrew Fresh afre...@openbsd.org wrote: Turns out in 2008 the regex that aliases ISO-646 to ascii was changed so that it also matches plain 646, so we no longer need a patch to do that. Makes sense. ok guenther@ The secondary quotes on the definition ('ascii') are due to the way regex aliasing works and get stripped off. OK to remove our local patch? -- andrew - http://afresh1.com ($do || !$do) undef($try) ; # Master of Perl, Yoda is. H? Index: gnu/usr.bin/perl/cpan/Encode/lib/Encode/Alias.pm === RCS file: /cvs/src/gnu/usr.bin/perl/cpan/Encode/lib/Encode/Alias.pm,v retrieving revision 1.4 diff -u -p -u -r1.4 Alias.pm --- gnu/usr.bin/perl/cpan/Encode/lib/Encode/Alias.pm24 Mar 2014 15:05:24 - 1.4 +++ gnu/usr.bin/perl/cpan/Encode/lib/Encode/Alias.pm31 May 2014 17:08:19 - @@ -152,7 +152,6 @@ sub init_aliases { define_alias( qr/^(?:US-?)ascii$/i = 'ascii' ); define_alias( 'C'= 'ascii' ); define_alias( qr/\b(?:ISO[-_]?)?646(?:[-_]?US)?$/i = 'ascii' ); -define_alias( '646' = 'ascii' ); # Allow variants of iso-8859-1 etc. define_alias( qr/\biso[-_]?(\d+)[-_](\d+)$/i = 'iso-$1-$2' );
[PATCH 0/7] libssl fixes
This patch set fixes a series of issues flagged by recent versions of clang and gcc. Do these look OK? - Brent
[PATCH 4/7] use BIO_write instead of an unchecked write()
write() warns if its return value is unchecked. Replace with a BIO_write like all of the surrounding code uses anyway. --- src/apps/s_server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/apps/s_server.c b/src/apps/s_server.c index 51f6b47..fb28489 100644 --- a/src/apps/s_server.c +++ b/src/apps/s_server.c @@ -1767,7 +1767,7 @@ sv_body(char *hostname, int s, unsigned char *context) i = SSL_read(con, (char *) buf, bufsize); switch (SSL_get_error(con, i)) { case SSL_ERROR_NONE: - write(fileno(stdout), buf, + BIO_write(bio_s_out, buf, (unsigned int) i); if (SSL_pending(con)) goto again; -- 1.9.3
[PATCH 1/7] If EVP_DecryptInit_ex() returns NULL, j is incremented by a random amount in PEM_do_header()
clang warning: pem/pem_lib.c:472:6: error: variable 'i' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] if (o) ^ pem/pem_lib.c:479:7: note: uninitialized use occurs here j += i; ^ pem/pem_lib.c:472:2: note: remove the 'if' if its condition is always true if (o) ^~ pem/pem_lib.c:446:7: note: initialize the variable 'i' to silence this warning int i, j, o, klen; --- src/crypto/pem/pem_lib.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/crypto/pem/pem_lib.c b/src/crypto/pem/pem_lib.c index 945262f..92c3dc4 100644 --- a/src/crypto/pem/pem_lib.c +++ b/src/crypto/pem/pem_lib.c @@ -454,6 +454,7 @@ PEM_do_header(EVP_CIPHER_INFO *cipher, unsigned char *data, long *plen, (unsigned char *)buf, klen, 1, key, NULL)) return 0; + i = 0; j = (int)len; EVP_CIPHER_CTX_init(ctx); o = EVP_DecryptInit_ex(ctx, cipher-cipher, NULL, key, -- 1.9.3
[PATCH 2/7] fix type string conversion warning
ASN1_STRING_data returns an unsigned char *, but strlcat's second parameter is a const char * --- src/crypto/ts/ts_rsp_verify.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crypto/ts/ts_rsp_verify.c b/src/crypto/ts/ts_rsp_verify.c index 2a4c0c5..49754b5 100644 --- a/src/crypto/ts/ts_rsp_verify.c +++ b/src/crypto/ts/ts_rsp_verify.c @@ -564,7 +564,7 @@ TS_get_status_text(STACK_OF(ASN1_UTF8STRING) *text) ASN1_UTF8STRING *current = sk_ASN1_UTF8STRING_value(text, i); if (i 0) strlcat(result, /, length); - strlcat(result, ASN1_STRING_data(current), length); + strlcat(result, (const char *)ASN1_STRING_data(current), length); } return result; } -- 1.9.3
[PATCH 3/7] remove unused static datastructures
Neither of these is used anywhere within their object files. --- src/crypto/ec/ec_lib.c | 3 --- src/crypto/engine/eng_dyn.c | 3 --- 2 files changed, 6 deletions(-) diff --git a/src/crypto/ec/ec_lib.c b/src/crypto/ec/ec_lib.c index 3313573..b8e0e98 100644 --- a/src/crypto/ec/ec_lib.c +++ b/src/crypto/ec/ec_lib.c @@ -68,9 +68,6 @@ #include ec_lcl.h -static const char EC_version[] = EC OPENSSL_VERSION_PTEXT; - - /* functions for EC_GROUP objects */ EC_GROUP * diff --git a/src/crypto/engine/eng_dyn.c b/src/crypto/engine/eng_dyn.c index e2de460..e6861a5 100644 --- a/src/crypto/engine/eng_dyn.c +++ b/src/crypto/engine/eng_dyn.c @@ -114,9 +114,6 @@ static const ENGINE_CMD_DEFN dynamic_cmd_defns[] = { ENGINE_CMD_FLAG_NO_INPUT}, {0, NULL, NULL, 0} }; -static const ENGINE_CMD_DEFN dynamic_cmd_defns_empty[] = { - {0, NULL, NULL, 0} - }; /* Loading code stores state inside the ENGINE structure via the ex_data * element. We load all our state into a single structure and use that as a -- 1.9.3
[PATCH 7/7] avoid defining struct pqueue typedef twice
with pqueue moving to a private interface, the typedef can occur twice ../include/openssl/dtls1.h:147:25: error: redefinition of typedef 'pqueue' is a C11 feature [-Werror,-Wtypedef-redefinition] typedef struct _pqueue *pqueue; ^ ../include/pqueue.h:63:25: note: previous definition is here typedef struct _pqueue *pqueue; --- src/ssl/dtls1.h | 3 +++ src/ssl/pqueue.h | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/ssl/dtls1.h b/src/ssl/dtls1.h index b0cf839..3dc2234 100644 --- a/src/ssl/dtls1.h +++ b/src/ssl/dtls1.h @@ -143,8 +143,11 @@ struct dtls1_timeout_st { unsigned int num_alerts; }; +#ifndef HAVE_STRUCT_PQUEUE +#define HAVE_STRUCT_PQUEUE struct _pqueue; typedef struct _pqueue *pqueue; +#endif typedef struct record_pqueue_st { unsigned short epoch; diff --git a/src/ssl/pqueue.h b/src/ssl/pqueue.h index 4b16787..78ce08f 100644 --- a/src/ssl/pqueue.h +++ b/src/ssl/pqueue.h @@ -60,7 +60,10 @@ #ifndef HEADER_PQUEUE_H #define HEADER_PQUEUE_H +#ifndef HAVE_STRUCT_PQUEUE +#define HAVE_STRUCT_PQUEUE typedef struct _pqueue *pqueue; +#endif typedef struct _pitem { unsigned char priority[8]; /* 64-bit value in big-endian encoding */ -- 1.9.3
[PATCH 0/2] libcrypto regress fixes
This is a series of fixes to issues found while building libcrypto regression tests with -Werror and LIBRESSL_INTERNAL. Do these look OK? - Brent
[PATCH 5/7] do not include public headers as though they are local
Avoid having to use -I trickery to find public header files included as though they are private. --- src/crypto/bn/bn_const.c | 2 +- src/crypto/chacha/chacha.c | 2 +- src/crypto/o_init.c| 2 +- src/crypto/poly1305/poly1305.c | 2 +- src/crypto/ts/ts_lib.c | 2 +- src/crypto/ts/ts_rsp_print.c | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/crypto/bn/bn_const.c b/src/crypto/bn/bn_const.c index 1c8bc9b..2ba2bbd 100644 --- a/src/crypto/bn/bn_const.c +++ b/src/crypto/bn/bn_const.c @@ -1,7 +1,7 @@ /* crypto/bn/knownprimes.c */ /* Insert boilerplate */ -#include bn.h +#include openssl/bn.h /* First Oakley Default Group from RFC2409, section 6.1. * diff --git a/src/crypto/chacha/chacha.c b/src/crypto/chacha/chacha.c index 1bc95f5..f0e818e 100644 --- a/src/crypto/chacha/chacha.c +++ b/src/crypto/chacha/chacha.c @@ -14,7 +14,7 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -#include chacha.h +#include openssl/chacha.h #include chacha-merged.c void diff --git a/src/crypto/o_init.c b/src/crypto/o_init.c index 09c9820..2c95a9e 100644 --- a/src/crypto/o_init.c +++ b/src/crypto/o_init.c @@ -1,5 +1,5 @@ /* Ted Unangst places this file in the public domain. */ -#include crypto.h +#include openssl/crypto.h void OPENSSL_init(void) diff --git a/src/crypto/poly1305/poly1305.c b/src/crypto/poly1305/poly1305.c index 96083fe..c2c3906 100644 --- a/src/crypto/poly1305/poly1305.c +++ b/src/crypto/poly1305/poly1305.c @@ -14,7 +14,7 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -#include poly1305.h +#include openssl/poly1305.h #include poly1305-donna.c void diff --git a/src/crypto/ts/ts_lib.c b/src/crypto/ts/ts_lib.c index b9d92a3..5018e95 100644 --- a/src/crypto/ts/ts_lib.c +++ b/src/crypto/ts/ts_lib.c @@ -61,7 +61,7 @@ #include openssl/objects.h #include openssl/bn.h #include openssl/x509v3.h -#include ts.h +#include openssl/ts.h /* Local function declarations. */ diff --git a/src/crypto/ts/ts_rsp_print.c b/src/crypto/ts/ts_rsp_print.c index 248674f..6615e20 100644 --- a/src/crypto/ts/ts_rsp_print.c +++ b/src/crypto/ts/ts_rsp_print.c @@ -61,7 +61,7 @@ #include openssl/objects.h #include openssl/bn.h #include openssl/x509v3.h -#include ts.h +#include openssl/ts.h struct status_map_st { int bit; -- 1.9.3
[PATCH 1/2] -Werror build fixes for aes_wrap.c
include string.h for memcmp remove unused variables --- aeswrap/aes_wrap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aeswrap/aes_wrap.c b/aeswrap/aes_wrap.c index b5157d7..c3079e3 100644 --- a/aeswrap/aes_wrap.c +++ b/aeswrap/aes_wrap.c @@ -10,7 +10,7 @@ * are met: * * 1. Redistributions of source code must retain the above copyright - *notice, this list of conditions and the following disclaimer. + *notice, this list of conditions and the following disclaimer. * * 2. Redistributions in binary form must reproduce the above copyright *notice, this list of conditions and the following disclaimer in @@ -53,6 +53,7 @@ #include stdio.h #include stdlib.h +#include string.h #include openssl/aes.h @@ -150,7 +151,6 @@ main(int argc, char **argv) 0xfb, 0x98, 0x8b, 0x9b, 0x7a, 0x02, 0xdd, 0x21 }; - AES_KEY wctx, xctx; int ret, nfailures = 0; ret = AES_wrap_unwrap_test(kek, 128, NULL, e1, key, 16); if (ret == 0) -- 1.9.3
[PATCH 2/2] remove uses of internally-deprecated OPENSSL_malloc/free
--- ecdh/ecdhtest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ecdh/ecdhtest.c b/ecdh/ecdhtest.c index 620424d..ee220dc 100644 --- a/ecdh/ecdhtest.c +++ b/ecdh/ecdhtest.c @@ -358,7 +358,7 @@ static int ecdh_kat(BIO *out, const char *cname, int nid, Ztmplen = (EC_GROUP_get_degree(EC_KEY_get0_group(key1)) + 7)/8; if (Ztmplen != Zlen) goto err; - Ztmp = OPENSSL_malloc(Ztmplen); + Ztmp = malloc(Ztmplen); if (!ECDH_compute_key(Ztmp, Ztmplen, EC_KEY_get0_public_key(key2), key1, 0)) goto err; @@ -377,7 +377,7 @@ static int ecdh_kat(BIO *out, const char *cname, int nid, if (key2) EC_KEY_free(key2); if (Ztmp) - OPENSSL_free(Ztmp); + free(Ztmp); if (rv) BIO_puts(out, ok\n); else -- 1.9.3
[PATCH 6/7] remove parsing of -rand options in openssl apps
Since the random number generator no longer allows being seeded, remove support for parsing the unused -rand option and the unused random buffer variables. Better to fail than to be surprised when the RNG seed does not function as expected. This fixes compiler warnings about unused random seed variables. --- src/apps/cms.c | 9 - src/apps/dgst.c | 8 ++-- src/apps/dhparam.c | 10 +- src/apps/dsaparam.c | 7 +-- src/apps/ecparam.c | 9 + src/apps/gendh.c| 10 +- src/apps/gendsa.c | 11 ++- src/apps/genrsa.c | 9 - src/apps/pkcs12.c | 10 -- src/apps/rand.c | 10 +- src/apps/req.c | 9 - src/apps/s_client.c | 8 +--- src/apps/s_server.c | 7 --- src/apps/smime.c| 10 -- src/apps/ts.c | 7 +-- 15 files changed, 11 insertions(+), 123 deletions(-) diff --git a/src/apps/cms.c b/src/apps/cms.c index 56a7c95..76178b4 100644 --- a/src/apps/cms.c +++ b/src/apps/cms.c @@ -127,7 +127,6 @@ cms_main(int argc, char **argv) char *to = NULL, *from = NULL, *subject = NULL; char *CAfile = NULL, *CApath = NULL; char *passargin = NULL, *passin = NULL; - char *inrand = NULL; const EVP_MD *sign_md = NULL; int informat = FORMAT_SMIME, outformat = FORMAT_SMIME; int rctformat = FORMAT_SMIME, keyform = FORMAT_PEM; @@ -315,11 +314,6 @@ cms_main(int argc, char **argv) BIO_printf(bio_err, Invalid OID %s\n, *args); goto argerr; } - } else if (!strcmp(*args, -rand)) { - if (!args[1]) - goto argerr; - args++; - inrand = *args; } #ifndef OPENSSL_NO_ENGINE else if (!strcmp(*args, -engine)) { @@ -553,9 +547,6 @@ argerr: BIO_printf(bio_err, -engine e use engine e, possibly a hardware device.\n); #endif BIO_printf(bio_err, -passin arginput file pass phrase source\n); - BIO_printf(bio_err, -rand file:file:...\n); - BIO_printf(bio_err,load the file (or the files in the directory) into\n); - BIO_printf(bio_err,the random number generator\n); BIO_printf(bio_err, cert.pem recipient certificate(s) for encryption\n); goto end; } diff --git a/src/apps/dgst.c b/src/apps/dgst.c index 23b7d40..a862da9 100644 --- a/src/apps/dgst.c +++ b/src/apps/dgst.c @@ -116,7 +116,7 @@ dgst_main(int argc, char **argv) int debug = 0; int keyform = FORMAT_PEM; const char *outfile = NULL, *keyfile = NULL; - const char *sigfile = NULL, *randfile = NULL; + const char *sigfile = NULL; int out_bin = -1, want_pub = 0, do_verify = 0; EVP_PKEY *sigkey = NULL; unsigned char *sigbuf = NULL; @@ -151,11 +151,7 @@ dgst_main(int argc, char **argv) separator = 1; else if (strcmp(*argv, -r) == 0) separator = 2; - else if (strcmp(*argv, -rand) == 0) { - if (--argc 1) - break; - randfile = *(++argv); - } else if (strcmp(*argv, -out) == 0) { + else if (strcmp(*argv, -out) == 0) { if (--argc 1) break; outfile = *(++argv); diff --git a/src/apps/dhparam.c b/src/apps/dhparam.c index 3245e69..c35f902 100644 --- a/src/apps/dhparam.c +++ b/src/apps/dhparam.c @@ -159,7 +159,6 @@ dhparam_main(int argc, char **argv) BIO *in = NULL, *out = NULL; int informat, outformat, check = 0, noout = 0, C = 0, ret = 1; char *infile, *outfile, *prog; - char *inrand = NULL; #ifndef OPENSSL_NO_ENGINE char *engine = NULL; #endif @@ -217,11 +216,7 @@ dhparam_main(int argc, char **argv) g = 2; else if (strcmp(*argv, -5) == 0) g = 5; - else if (strcmp(*argv, -rand) == 0) { - if (--argc 1) - goto bad; - inrand = *(++argv); - } else if (((sscanf(*argv, %d, num) == 0) || (num = 0))) + else if (((sscanf(*argv, %d, num) == 0) || (num = 0))) goto bad; argv++; argc--; @@ -247,9 +242,6 @@ bad: #ifndef OPENSSL_NO_ENGINE BIO_printf(bio_err, -engine e use engine e, possibly a hardware device.\n); #endif - BIO_printf(bio_err, -rand file:file:...\n); - BIO_printf(bio_err,- load the file (or the files in the directory) into\n); - BIO_printf(bio_err,the
Re: [PATCH 4/7] use BIO_write instead of an unchecked write()
Your change is wrong. There have been lots of discussion about trying to use intrinsics as much as possible. Even though this is the openssl command, I think the consideration is also valid here. You've just run into the reason for using intrinsics as much as possible. BIO_write is different from write. See how insane this is? Anyways, the compiler is telling you to check the return value. Why don't you check it, and put in some effort determining what action to take in that case? The result will be better. write() warns if its return value is unchecked. Replace with a BIO_write like all of the surrounding code uses anyway. --- src/apps/s_server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/apps/s_server.c b/src/apps/s_server.c index 51f6b47..fb28489 100644 --- a/src/apps/s_server.c +++ b/src/apps/s_server.c @@ -1767,7 +1767,7 @@ sv_body(char *hostname, int s, unsigned char *context) i = SSL_read(con, (char *) buf, bufsize); switch (SSL_get_error(con, i)) { case SSL_ERROR_NONE: - write(fileno(stdout), buf, + BIO_write(bio_s_out, buf, (unsigned int) i); if (SSL_pending(con)) goto again; -- 1.9.3
Re: [PATCH 2/2] remove uses of internally-deprecated OPENSSL_malloc/free
You should change this so it doesnt check for null before the free. free handles NULL. On 31 May 2014 16:39, Brent Cook bust...@gmail.com wrote: --- ecdh/ecdhtest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ecdh/ecdhtest.c b/ecdh/ecdhtest.c index 620424d..ee220dc 100644 --- a/ecdh/ecdhtest.c +++ b/ecdh/ecdhtest.c @@ -358,7 +358,7 @@ static int ecdh_kat(BIO *out, const char *cname, int nid, Ztmplen = (EC_GROUP_get_degree(EC_KEY_get0_group(key1)) + 7)/8; if (Ztmplen != Zlen) goto err; - Ztmp = OPENSSL_malloc(Ztmplen); + Ztmp = malloc(Ztmplen); if (!ECDH_compute_key(Ztmp, Ztmplen, EC_KEY_get0_public_key(key2), key1, 0)) goto err; @@ -377,7 +377,7 @@ static int ecdh_kat(BIO *out, const char *cname, int nid, if (key2) EC_KEY_free(key2); if (Ztmp) - OPENSSL_free(Ztmp); + free(Ztmp); if (rv) BIO_puts(out, ok\n); else -- 1.9.3
Re: [PATCH 4/7] use BIO_write instead of an unchecked write()
I totally agree in the general case. BIO is a big pain, and it does seem crazy to use it for stdio. However, in this specific case, this file already calls BIO_printf, BIO_puts and BIO_write to stdout and stderr, in an unchecked manner, several hundred times. I’m not sure if checking write() actually fixes the problem, as there may be no guarantees of IO ordering when mixing BIO_write, BIO_puts and regular write() calls to the same output stream. Might it be OK to consider unifying this single exception to the (bad) rule, and then removing all BIO to stdin/stdout all at once in a follow-up patch? On May 31, 2014, at 5:40 PM, Theo de Raadt dera...@cvs.openbsd.org wrote: Your change is wrong. There have been lots of discussion about trying to use intrinsics as much as possible. Even though this is the openssl command, I think the consideration is also valid here. You've just run into the reason for using intrinsics as much as possible. BIO_write is different from write. See how insane this is? Anyways, the compiler is telling you to check the return value. Why don't you check it, and put in some effort determining what action to take in that case? The result will be better. write() warns if its return value is unchecked. Replace with a BIO_write like all of the surrounding code uses anyway. --- src/apps/s_server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/apps/s_server.c b/src/apps/s_server.c index 51f6b47..fb28489 100644 --- a/src/apps/s_server.c +++ b/src/apps/s_server.c @@ -1767,7 +1767,7 @@ sv_body(char *hostname, int s, unsigned char *context) i = SSL_read(con, (char *) buf, bufsize); switch (SSL_get_error(con, i)) { case SSL_ERROR_NONE: -write(fileno(stdout), buf, +BIO_write(bio_s_out, buf, (unsigned int) i); if (SSL_pending(con)) goto again; -- 1.9.3
Remove unneeded workaround for usemallocwrap on m68k in perl
This local patch should longer be needed with the removal of m68k. OK? Index: hints/openbsd.sh === RCS file: /cvs/src/gnu/usr.bin/perl/hints/openbsd.sh,v retrieving revision 1.49 diff -u -p -u -r1.49 openbsd.sh --- hints/openbsd.sh24 Mar 2014 15:05:27 - 1.49 +++ hints/openbsd.sh31 May 2014 22:59:30 - @@ -11,6 +11,11 @@ # OpenBSD has a better malloc than perl... test $usemymalloc || usemymalloc='n' +# malloc wrap works +case $usemallocwrap in +'') usemallocwrap='define' ;; +esac + # Currently, vfork(2) is not a real win over fork(2). usevfork=$undef @@ -74,14 +79,6 @@ case $osvers in i_dbm=$undef ;; esac - -# malloc wrap causes problems on m68k -if [ X$usemallocwrap = X ]; then - case ${ARCH} in - m68k) usemallocwrap=$undef ;; - *)usemallocwrap=define ;; - esac -fi # OpenBSD doesn't need libcrypt but many folks keep a stub lib # around for old NetBSD binaries.
Re: [PATCH 4/7] use BIO_write instead of an unchecked write()
On Sat, May 31, 2014 at 8:02 PM, Brent Cook bust...@gmail.com wrote: I totally agree in the general case. BIO is a big pain, and it does seem crazy to use it for stdio. However, in this specific case, this file already calls BIO_printf, BIO_puts and BIO_write to stdout and stderr, in an unchecked manner, several hundred times. I’m not sure if checking write() actually fixes the problem, as there may be no guarantees of IO ordering when mixing BIO_write, BIO_puts and regular write() calls to the same output stream. Might it be OK to consider unifying this single exception to the (bad) rule, and then removing all BIO to stdin/stdout all at once in a follow-up patch? For what it's worth, bio_s_out is not stdout if -quiet is passed (it's a BIO_s_null instead.) If you change this write to BIO_write, then it will be suppressed as well.
Correct OpenBSD versions for disabling getserbyname_r
So, in r1.40 the comment was corrected to reflect that this was fixed in 3.7, but the actual check still only checked up to 3.5. http://www.openbsd.org/cgi-bin/cvsweb/src/gnu/usr.bin/perl/hints/openbsd.sh.diff?r1=1.39;r2=1.40;f=h And then r1.1.1.8 was created with the correct version in the check, but an incorrect comment. http://www.openbsd.org/cgi-bin/cvsweb/src/gnu/usr.bin/perl/hints/openbsd.sh.diff?r1=1.1.1.7;r2=1.1.1.8;f=h r1.45 brings it back the correct comment, but unfortunately the incorrect check. http://www.openbsd.org/cgi-bin/cvsweb/src/gnu/usr.bin/perl/hints/openbsd.sh.diff?r1=1.44;r2=1.45;f=h This brings back the correct version of the check and leaves the reasonable comment. I will be sending this patch upstream when it makes more sense. https://github.com/afresh1/OpenBSD-perl/blob/master/patches/GOOD/UPSTREAM/correct_versions_in_hints.patch OK? Index: gnu/usr.bin/perl/hints/openbsd.sh === RCS file: /cvs/src/gnu/usr.bin/perl/hints/openbsd.sh,v retrieving revision 1.51 diff -u -p -u -r1.51 openbsd.sh --- gnu/usr.bin/perl/hints/openbsd.sh 31 May 2014 23:16:21 - 1.51 +++ gnu/usr.bin/perl/hints/openbsd.sh 1 Jun 2014 00:24:17 - @@ -118,7 +118,7 @@ $define|true|[yY]*) ;; esac case $osvers in - [012].*|3.[0-5]) + [012].*|3.[0-6]) # Broken up to OpenBSD 3.6, fixed in OpenBSD 3.7 d_getservbyname_r=$undef ;; esac
Re: [PATCH 4/7] use BIO_write instead of an unchecked write()
On May 31, 2014, at 6:08 PM, Brendan MacDonell macdonel...@gmail.com wrote: On Sat, May 31, 2014 at 8:02 PM, Brent Cook bust...@gmail.com wrote: I totally agree in the general case. BIO is a big pain, and it does seem crazy to use it for stdio. However, in this specific case, this file already calls BIO_printf, BIO_puts and BIO_write to stdout and stderr, in an unchecked manner, several hundred times. I’m not sure if checking write() actually fixes the problem, as there may be no guarantees of IO ordering when mixing BIO_write, BIO_puts and regular write() calls to the same output stream. Might it be OK to consider unifying this single exception to the (bad) rule, and then removing all BIO to stdin/stdout all at once in a follow-up patch? For what it's worth, bio_s_out is not stdout if -quiet is passed (it's a BIO_s_null instead.) If you change this write to BIO_write, then it will be suppressed as well. Good point. The help for this command says: -quiet- No server output It seems like supressing here would be the correct behavior as well. Maybe a larger question is, should the openssl command really embed a static-file-serving HTTP 1.0 web server? The ‘-hack’ and ‘-bugs’ options seem like a little ripe, among many other things. - Brent
Re: [PATCH 1/7] If EVP_DecryptInit_ex() returns NULL, j is incremented by a random amount in PEM_do_header()
On Sun, 1 Jun 2014, Brent Cook wrote: clang warning: pem/pem_lib.c:472:6: error: variable 'i' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] if (o) ^ pem/pem_lib.c:479:7: note: uninitialized use occurs here j += i; ^ pem/pem_lib.c:472:2: note: remove the 'if' if its condition is always true if (o) ^~ pem/pem_lib.c:446:7: note: initialize the variable 'i' to silence this warning int i, j, o, klen; --- src/crypto/pem/pem_lib.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/crypto/pem/pem_lib.c b/src/crypto/pem/pem_lib.c index 945262f..92c3dc4 100644 --- a/src/crypto/pem/pem_lib.c +++ b/src/crypto/pem/pem_lib.c @@ -454,6 +454,7 @@ PEM_do_header(EVP_CIPHER_INFO *cipher, unsigned char *data, long *plen, (unsigned char *)buf, klen, 1, key, NULL)) return 0; + i = 0; j = (int)len; EVP_CIPHER_CTX_init(ctx); o = EVP_DecryptInit_ex(ctx, cipher-cipher, NULL, key, This is a non-issue since the value of j is unused in the !o case. That said, I've just commited the following diff actually fixes the code, rather than just addressing the uninitialised variable: Index: pem_lib.c === RCS file: /cvs/src/lib/libssl/src/crypto/pem/pem_lib.c,v retrieving revision 1.23 diff -u -p -r1.23 pem_lib.c --- pem_lib.c 26 Apr 2014 18:56:38 - 1.23 +++ pem_lib.c 29 May 2014 15:39:26 - @@ -476,12 +476,11 @@ PEM_do_header(EVP_CIPHER_INFO *cipher, u EVP_CIPHER_CTX_cleanup(ctx); OPENSSL_cleanse((char *)buf, sizeof(buf)); OPENSSL_cleanse((char *)key, sizeof(key)); - j += i; if (!o) { PEMerr(PEM_F_PEM_DO_HEADER, PEM_R_BAD_DECRYPT); return (0); } - *plen = j; + *plen = j + i; return (1); } -- Action without study is fatal. Study without action is futile. -- Mary Ritter Beard