Re: [patch ping.c] replace malloc & memset with calloc
On Tue, Apr 22, 2014 at 02:57:54AM -0400, pe...@petermalone.org wrote:
> Sure - I should have spotted that.
Still not there. Please use the fact that calloc can multiply, you get
an overflow check for free.
-Otto
>
> Index: ping.c
> ===
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.100
> diff -u -r1.100 ping.c
> --- ping.c 24 Mar 2014 11:11:49 - 1.100
> +++ ping.c 22 Apr 2014 06:55:03 -
> @@ -188,7 +188,6 @@
> socklen_t maxsizelen;
> const char *errstr;
> fd_set *fdmaskp;
> - size_t fdmasks;
> uid_t uid;
> u_int rtableid;
>
> @@ -507,9 +506,8 @@
> if ((options & F_FLOOD) == 0)
> catcher(0); /* start things going */
>
> - fdmasks = howmany(s+1, NFDBITS) * sizeof(fd_mask);
> - if ((fdmaskp = (fd_set *)malloc(fdmasks)) == NULL)
> - err(1, "malloc");
> + if ((fdmaskp = calloc(1, howmany(s+1, NFDBITS) * sizeof(fd_mask)))
> == NULL)
> + err(1, "calloc");
>
> for (;;) {
> struct sockaddr_in from;
> @@ -521,7 +519,6 @@
> pinger();
> timeout.tv_sec = 0;
> timeout.tv_usec = 1;
> - memset(fdmaskp, 0, fdmasks);
> FD_SET(s, fdmaskp);
> if (select(s + 1, (fd_set *)fdmaskp, (fd_set *)NULL,
> (fd_set *)NULL, &timeout) < 1)
>
>
> Quoting Otto Moerbeek :
>
> >On Tue, Apr 22, 2014 at 12:45:25AM -0400, Peter Malone wrote:
> >
> >>Hi,
> >>
> >>malloc & memset can be replaced with calloc in ping.c. Please see below for
> >>patch details:
> >
> >Better rework this to get rid of fdmasks.
> >
> > -Otto
> >
> >>
> >>Index: ping.c
> >>===
> >>RCS file: /cvs/src/sbin/ping/ping.c,v
> >>retrieving revision 1.100
> >>diff -u -p -u -r1.100 ping.c
> >>--- ping.c24 Mar 2014 11:11:49 -1.100
> >>+++ ping.c22 Apr 2014 04:41:56 -
> >>@@ -508,8 +508,8 @@ main(int argc, char *argv[])
> >> catcher(0);/* start things going */
> >>
> >> fdmasks = howmany(s+1, NFDBITS) * sizeof(fd_mask);
> >>-if ((fdmaskp = (fd_set *)malloc(fdmasks)) == NULL)
> >>-err(1, "malloc");
> >>+if ((fdmaskp = calloc(1, fdmasks)) == NULL)
> >>+err(1, "calloc");
> >>
> >> for (;;) {
> >> struct sockaddr_in from;
> >>@@ -521,7 +521,6 @@ main(int argc, char *argv[])
> >> pinger();
> >> timeout.tv_sec = 0;
> >> timeout.tv_usec = 1;
> >>-memset(fdmaskp, 0, fdmasks);
> >> FD_SET(s, fdmaskp);
> >> if (select(s + 1, (fd_set *)fdmaskp, (fd_set *)NULL,
> >> (fd_set *)NULL, &timeout) < 1)
> >>
> >
> >
>
Re: 12 seconds delay when starting X.org
Il giorno 22/apr/2014 06.59, "Kārlis Miķelsons" ha scritto: > > Hello, > > >> Your DNS is broken. xauth is trying to resolve the current hostname. >> Fix DNS or add something to /etc/hosts. > > Thank you for your suggestion, but it didn't help. > > $ cat /etc/resolv.conf > family inet4 inet6 > nameserver 8.8.8.8 > lookup file bind > > $ hostname > host.domain.com > > $ host host.domain.com > host.domain.com has address 127.0.0.1 > > $ cat /etc/hosts | grep -v "#" > 127.0.0.1 localhost > ::1 localhost > 127.0.0.1 host host.domain.com > > I also made sure that reverse DNS for my IP address works fine and > without any delays and tried multiple different DNS servers in > resolv.conf It's the pms(4) driver. It happens on some Dell laptops that have a crappy non fully standard synaptics (alps?) touchpad.
Re: 12 seconds delay when starting X.org
It's the pms(4) driver. It happens on some Dell laptops that have a crappy non fully standard synaptics (alps?) touchpad. It seems so, today this laptop booted up without pms device for some reason, and without it X.org started up instantly. -- Karlis
Re: [patch ping.c] replace malloc & memset with calloc
Please switch it to poll(2) like ping6(8) is doing, there by side
stepping the whole issue.
On Tue, Apr 22, 2014 at 09:33:50AM +0200, Otto Moerbeek wrote:
> On Tue, Apr 22, 2014 at 02:57:54AM -0400, pe...@petermalone.org wrote:
>
> > Sure - I should have spotted that.
>
> Still not there. Please use the fact that calloc can multiply, you get
> an overflow check for free.
>
> -Otto
>
> >
> > Index: ping.c
> > ===
> > RCS file: /cvs/src/sbin/ping/ping.c,v
> > retrieving revision 1.100
> > diff -u -r1.100 ping.c
> > --- ping.c 24 Mar 2014 11:11:49 - 1.100
> > +++ ping.c 22 Apr 2014 06:55:03 -
> > @@ -188,7 +188,6 @@
> > socklen_t maxsizelen;
> > const char *errstr;
> > fd_set *fdmaskp;
> > - size_t fdmasks;
> > uid_t uid;
> > u_int rtableid;
> >
> > @@ -507,9 +506,8 @@
> > if ((options & F_FLOOD) == 0)
> > catcher(0); /* start things going */
> >
> > - fdmasks = howmany(s+1, NFDBITS) * sizeof(fd_mask);
> > - if ((fdmaskp = (fd_set *)malloc(fdmasks)) == NULL)
> > - err(1, "malloc");
> > + if ((fdmaskp = calloc(1, howmany(s+1, NFDBITS) * sizeof(fd_mask)))
> > == NULL)
> > + err(1, "calloc");
> >
> > for (;;) {
> > struct sockaddr_in from;
> > @@ -521,7 +519,6 @@
> > pinger();
> > timeout.tv_sec = 0;
> > timeout.tv_usec = 1;
> > - memset(fdmaskp, 0, fdmasks);
> > FD_SET(s, fdmaskp);
> > if (select(s + 1, (fd_set *)fdmaskp, (fd_set *)NULL,
> > (fd_set *)NULL, &timeout) < 1)
> >
> >
> > Quoting Otto Moerbeek :
> >
> > >On Tue, Apr 22, 2014 at 12:45:25AM -0400, Peter Malone wrote:
> > >
> > >>Hi,
> > >>
> > >>malloc & memset can be replaced with calloc in ping.c. Please see below
> > >>for
> > >>patch details:
> > >
> > >Better rework this to get rid of fdmasks.
> > >
> > > -Otto
> > >
> > >>
> > >>Index: ping.c
> > >>===
> > >>RCS file: /cvs/src/sbin/ping/ping.c,v
> > >>retrieving revision 1.100
> > >>diff -u -p -u -r1.100 ping.c
> > >>--- ping.c24 Mar 2014 11:11:49 -1.100
> > >>+++ ping.c22 Apr 2014 04:41:56 -
> > >>@@ -508,8 +508,8 @@ main(int argc, char *argv[])
> > >> catcher(0);/* start things going */
> > >>
> > >> fdmasks = howmany(s+1, NFDBITS) * sizeof(fd_mask);
> > >>-if ((fdmaskp = (fd_set *)malloc(fdmasks)) == NULL)
> > >>-err(1, "malloc");
> > >>+if ((fdmaskp = calloc(1, fdmasks)) == NULL)
> > >>+err(1, "calloc");
> > >>
> > >> for (;;) {
> > >> struct sockaddr_in from;
> > >>@@ -521,7 +521,6 @@ main(int argc, char *argv[])
> > >> pinger();
> > >> timeout.tv_sec = 0;
> > >> timeout.tv_usec = 1;
> > >>-memset(fdmaskp, 0, fdmasks);
> > >> FD_SET(s, fdmaskp);
> > >> if (select(s + 1, (fd_set *)fdmaskp, (fd_set *)NULL,
> > >> (fd_set *)NULL, &timeout) < 1)
> > >>
> > >
> > >
> >
>
--
I'm not entirely sure you are real.
Re: 12 seconds delay when starting X.org
On Tue, Apr 22, 2014 at 10:29 AM, Kārlis Miķelsons wrote: >> It's the pms(4) driver. It happens on some Dell laptops that have a crappy >> non fully standard synaptics (alps?) touchpad. > > It seems so, today this laptop booted up without pms device for some reason, > and without it X.org started up instantly. Try to disable pms: # config -e -f /bsd UKC>disable pms And see if you can reproduce the problem.
Re: [patch ping.c] replace malloc & memset with calloc
On Tue, Apr 22, 2014 at 12:45:25AM -0400, Peter Malone wrote:
> Hi,
>
> malloc & memset can be replaced with calloc in ping.c. Please see below for
> patch details:
>
> Index: ping.c
> ===
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.100
> diff -u -p -u -r1.100 ping.c
> --- ping.c24 Mar 2014 11:11:49 -1.100
> +++ ping.c22 Apr 2014 04:41:56 -
> @@ -508,8 +508,8 @@ main(int argc, char *argv[])
> catcher(0);/* start things going */
>
> fdmasks = howmany(s+1, NFDBITS) * sizeof(fd_mask);
> -if ((fdmaskp = (fd_set *)malloc(fdmasks)) == NULL)
> -err(1, "malloc");
> +if ((fdmaskp = calloc(1, fdmasks)) == NULL)
> +err(1, "calloc");
>
> for (;;) {
> struct sockaddr_in from;
> @@ -521,7 +521,6 @@ main(int argc, char *argv[])
> pinger();
> timeout.tv_sec = 0;
> timeout.tv_usec = 1;
> -memset(fdmaskp, 0, fdmasks);
> FD_SET(s, fdmaskp);
> if (select(s + 1, (fd_set *)fdmaskp, (fd_set *)NULL,
> (fd_set *)NULL, &timeout) < 1)
>
I'm not sure if we should do this. The memset() call is in the select loop
and ensures that the bits in the fdmask are reset. This may not mater for
this case but needed in the general way selecte loops are done.
Also check the select man page for a proper way to do fdset allocation.
--
:wq Claudio
Re: more axeing at openssl
> This replaces RAND_{,pseudo_}bytes() calls with equivelant arc4random_buf(3)
> calls for apps/ and ssl/ (crypto/ still has a bunch).
Actually last time this was discussed, the idea was to leave this as-is for
now. Then we can decide if the "stronger of the two" should remain a seperate
name. There's all sorts of long-term consequences from prematurely folding
this.
Re: [patch ping.c] replace malloc & memset with calloc
You are now clearing only the first time.
> malloc & memset can be replaced with calloc in ping.c. Please see below
> for patch details:
>
> Index: ping.c
> ===
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.100
> diff -u -p -u -r1.100 ping.c
> --- ping.c24 Mar 2014 11:11:49 -1.100
> +++ ping.c22 Apr 2014 04:41:56 -
> @@ -508,8 +508,8 @@ main(int argc, char *argv[])
> catcher(0);/* start things going */
>
> fdmasks = howmany(s+1, NFDBITS) * sizeof(fd_mask);
> -if ((fdmaskp = (fd_set *)malloc(fdmasks)) == NULL)
> -err(1, "malloc");
> +if ((fdmaskp = calloc(1, fdmasks)) == NULL)
> +err(1, "calloc");
>
> for (;;) {
> struct sockaddr_in from;
> @@ -521,7 +521,6 @@ main(int argc, char *argv[])
> pinger();
> timeout.tv_sec = 0;
> timeout.tv_usec = 1;
> -memset(fdmaskp, 0, fdmasks);
> FD_SET(s, fdmaskp);
> if (select(s + 1, (fd_set *)fdmaskp, (fd_set *)NULL,
> (fd_set *)NULL, &timeout) < 1)
>
>
Re: IPv6 DoS sysctl man page additions
On Mon, Apr 21, 2014 at 09:39:55PM -0300, Fernando Gont wrote: > Hi, Loganaden, > > NetBSD really had these? I seem to recall that OpenBSD was the only BSD > variant with these (sensible) knobs. > > Thanks, > Fernando > They copied it from OpenBSD in 2012: kernel: Add sysctls to avoid ipv6 DoS attacks (from OpenBSD): net.inet6.ip6.neighborgcthresh = 2048 net.inet6.ip6.maxifprefixes = 16 net.inet6.ip6.maxifdefrouters = 16 net.inet6.ip6.maxdynroutes = 4096 [christos 20120622] > > > > On 04/19/2014 08:04 AM, Loganaden Velvindron wrote: > > Hi All, > > > > I'm taking a short break from playing with pf statistics. > > > > There were 4 sysctls added from KAME, but the man pages weren't updated > > accordingly. > > > > (Adapted from the NetBSD man page changes) > > > > Feedback welcomed. > > > > > > Index: lib/libc/gen/sysctl.3 > > === > > RCS file: /cvs/src/lib/libc/gen/sysctl.3,v > > retrieving revision 1.228 > > diff -u -p -u -p -r1.228 sysctl.3 > > --- lib/libc/gen/sysctl.3 21 Jan 2014 03:15:45 - 1.228 > > +++ lib/libc/gen/sysctl.3 19 Apr 2014 10:58:30 - > > @@ -1676,11 +1676,15 @@ The currently defined protocols and name > > .It ip6 Ta hdrnestlimit Ta integer Ta yes > > .It ip6 Ta hlim Ta integer Ta yes > > .It ip6 Ta log_interval Ta integer Ta yes > > +.It ip6 Ta maxdynroutes Ta integer Ta yes > > .It ip6 Ta maxfragpackets Ta integer Ta yes > > .It ip6 Ta maxfrags Ta integer Ta yes > > +.It ip6 Ta maxifprefixes Ta integer Ta yes > > +.It ip6 Ta maxifdefrouters Ta integer Ta yes > > .It ip6 Ta mforwarding Ta integer Ta yes > > .It ip6 Ta multicast_mtudisc Ta integer Ta yes > > .It ip6 Ta multipath Ta integer Ta yes > > +.It ip6 Ta neighborgcthresh Ta integer Ta yes > > .It ip6 Ta redirect Ta integer Ta yes > > .It ip6 Ta rr_prune Ta integer Ta yes > > .It ip6 Ta use_deprecated Ta integer Ta yes > > @@ -1834,6 +1838,11 @@ IPv6 packet forwarding engine. > > The value indicates the number of > > seconds of interval which must elapse between log output. > > .Pp > > +.It Li ip6.maxdynroutes > > +Maximum number of routes created by redirect. > > +Set it to negative to disable. > > +The default value is 4096. > > +.Pp > > .It Li ip6.maxfragpackets > > The maximum number of fragmented packets the node will accept. > > 0 means that the node will not accept any fragmented packets. > > @@ -1846,6 +1855,17 @@ The maximum number of fragments the node > > \-1 means that the node will accept as many fragments as it receives. > > The flag is provided basically for avoiding possible DoS attacks. > > .Pp > > +.It Li ip6.maxifprefixes > > +Maximum number of prefixes created by route advertisements per interface. > > +Set it to negative to disable. > > +The default value is 16. > > +.Pp > > +.It Li ip6.maxifdefrouters 16 > > +Maximum number of default routers created by route advertisements per > > +interface. > > +Set it to negative to disable. > > +The default value is 16. > > +.Pp > > .It Li ip6.mforwarding > > If set to 1, then multicast forwarding is enabled for the host. > > The default is 0. > > @@ -1861,6 +1881,11 @@ If set to 0, the ICMPv6 Too Big message > > This variable enables multipath routing for IPv6 addresses. > > If set to 0, only the first route selected will be used for a given > > destination regardless of how many routes exist in the routing table. > > +.Pp > > +.It Li ip6.neighborgcthresh > > +Maximum number of entries in neighbor cache. > > +Set to negative to disable. > > +The default value is 2048. > > .Pp > > .It Li ip6.redirect > > Returns 1 when ICMPv6 redirects may be sent by the node. > > Index: sbin/sysctl/sysctl.8 > > === > > RCS file: /cvs/src/sbin/sysctl/sysctl.8,v > > retrieving revision 1.173 > > diff -u -p -u -p -r1.173 sysctl.8 > > --- sbin/sysctl/sysctl.828 Oct 2013 21:02:35 - 1.173 > > +++ sbin/sysctl/sysctl.819 Apr 2014 10:58:30 - > > @@ -301,10 +301,14 @@ and a few require a kernel compiled with > > .It net.inet6.ip6.use_deprecated Ta integer Ta yes > > .It net.inet6.ip6.rr_prune Ta integer Ta yes > > .It net.inet6.ip6.v6only Ta integer Ta no > > +.It net.inet6.ip6.maxdynroutes Ta integer Ta yes > > .It net.inet6.ip6.maxfrags Ta integer Ta yes > > +.It net.inet6.ip6.maxifprefixes Ta integer Ta yes > > +.It net.inet6.ip6.maxifdefrouters Ta integer Ta yes > > .It net.inet6.ip6.mforwarding Ta integer Ta yes > > .It net.inet6.ip6.multipath Ta integer Ta yes > > .It net.inet6.ip6.multicast_mtudisc Ta integer Ta yes > > +.It net.inet6.ip6.neighborgcthresh Ta integer Ta yes > > .It net.inet6.icmp6.rediraccept Ta integer Ta yes > > .It net.inet6.icmp6.redirtimeout Ta integer Ta yes > > .It net.inet6.icmp6.nd6_prune Ta integer Ta yes > > > > > > > -- > Fernando Gont >
Re: IPv6 DoS sysctl man page additions
On 19 April 2014 13:20, Loganaden Velvindron wrote: > On Sat, Apr 19, 2014 at 04:04:30AM -0700, Loganaden Velvindron wrote: >> Hi All, >> >> I'm taking a short break from playing with pf statistics. >> >> There were 4 sysctls added from KAME, but the man pages weren't updated >> accordingly. >> >> (Adapted from the NetBSD man page changes) >> >> Feedback welcomed. >> >> > > Removed trailing spaces and use "set to" instead of "set it to" based > on feedback from sthen@ > > OK mikeb
Re: 12 seconds delay when starting X.org
It's the pms(4) driver. It happens on some Dell laptops that have a crappy non fully standard synaptics (alps?) touchpad. It seems so, today this laptop booted up without pms device for some reason, and without it X.org started up instantly. Try to disable pms: # config -e -f /bsd UKC>disable pms And see if you can reproduce the problem. I can, after disabling pms device, X.org starts just fine. dmesg: OpenBSD 5.5-current (GENERIC.MP) #74: Sun Apr 20 04:16:25 MDT 2014 dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP RTC BIOS diagnostic error 30 real mem = 4133875712 (3942MB) avail mem = 4015112192 (3829MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.6 @ 0xf2450 (66 entries) bios0: vendor Dell Inc. version "A14" date 12/05/2013 bios0: Dell Inc. Latitude E4310 acpi0 at bios0: rev 2 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP APIC TCPA MCFG HPET BOOT SLIC SSDT acpi0: wakeup devices AGP_(S4) P0P1(S4) UAR1(S3) HDEF(S4) PXSX(S4) RP01(S4) PXSX(S4) RP02(S4) PXSX(S4) RP03(S4) PXSX(S4) RP04(S4) PXSX(S4) RP05(S4) PXSX(S4) RP07(S4) [...] acpitimer0 at acpi0: 3579545 Hz, 24 bits acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Core(TM) i5 CPU M 560 @ 2.67GHz, 2926.51 MHz cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,POPCNT,AES,NXE,LONG,LAHF,PERF,ITSC cpu0: 256KB 64b/line 8-way L2 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges cpu0: apic clock running at 133MHz cpu0: mwait min=64, max=64, C-substates=0.2.1.1.0, IBE cpu1 at mainbus0: apid 4 (application processor) cpu1: Intel(R) Core(TM) i5 CPU M 560 @ 2.67GHz, 2926.01 MHz cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,POPCNT,AES,NXE,LONG,LAHF,PERF,ITSC cpu1: 256KB 64b/line 8-way L2 cache cpu1: smt 0, core 2, package 0 ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 24 pins acpimcfg0 at acpi0 addr 0xf800, bus 0-63 acpihpet0 at acpi0: 14318179 Hz acpiprt0 at acpi0: bus 0 (PCI0) acpiprt1 at acpi0: bus -1 (AGP_) acpiprt2 at acpi0: bus 3 (P0P1) acpiprt3 at acpi0: bus 1 (RP01) acpiprt4 at acpi0: bus -1 (RP02) acpiprt5 at acpi0: bus 2 (RP03) acpiprt6 at acpi0: bus -1 (RP04) acpiprt7 at acpi0: bus -1 (RP05) acpiprt8 at acpi0: bus -1 (RP07) acpiprt9 at acpi0: bus -1 (RP08) acpiprt10 at acpi0: bus -1 (PEG3) acpiprt11 at acpi0: bus -1 (PEG5) acpiec0 at acpi0 acpicpu0 at acpi0: C3, C1, PSS acpicpu1 at acpi0: C3, C1, PSS acpibtn0 at acpi0: LID_ acpibtn1 at acpi0: PBTN acpibtn2 at acpi0: SBTN acpiac0 at acpi0: AC unit online acpibat0 at acpi0: BAT0 model "DELL PXD3611" serial 36834 type LION oem "Samsung SDI" acpibat1 at acpi0: BAT1 not present acpivideo0 at acpi0: VID_ acpivout0 at acpivideo0: LCD_ cpu0: Enhanced SpeedStep 2926 MHz: speeds: 2667, 2666, 2533, 2399, 2266, 2133, 1999, 1866, 1733, 1599, 1466, 1333, 1199 MHz pci0 at mainbus0 bus 0 pchb0 at pci0 dev 0 function 0 "Intel Core Host" rev 0x02 vga1 at pci0 dev 2 function 0 "Intel HD Graphics" rev 0x02 intagp0 at vga1 agp0 at intagp0: aperture at 0xe000, size 0x1000 inteldrm0 at vga1 drm0 at inteldrm0 inteldrm0: 1366x768 wsdisplay0 at vga1 mux 1: console (std, vt100 emulation) wsdisplay0: screen 1-5 added (std, vt100 emulation) em0 at pci0 dev 25 function 0 "Intel 82577LM" rev 0x05: msi, address 5c:26:0a:3e:bd:bc ehci0 at pci0 dev 26 function 0 "Intel 3400 USB" rev 0x05: apic 2 int 16 usb0 at ehci0: USB revision 2.0 uhub0 at usb0 "Intel EHCI root hub" rev 2.00/1.00 addr 1 azalia0 at pci0 dev 27 function 0 "Intel QS57 HD Audio" rev 0x05: msi azalia0: codecs: IDT 92HD81B1X, Intel/0x2804, using IDT 92HD81B1X audio0 at azalia0 ppb0 at pci0 dev 28 function 0 "Intel 3400 PCIE" rev 0x05: msi pci1 at ppb0 bus 1 ppb1 at pci0 dev 28 function 2 "Intel 3400 PCIE" rev 0x05: msi pci2 at ppb1 bus 2 sdhc0 at pci2 dev 0 function 0 "Ricoh 5U822 SD/MMC" rev 0x01: apic 2 int 18 sdmmc0 at sdhc0 ehci1 at pci0 dev 29 function 0 "Intel 3400 USB" rev 0x05: apic 2 int 17 usb1 at ehci1: USB revision 2.0 uhub1 at usb1 "Intel EHCI root hub" rev 2.00/1.00 addr 1 ppb2 at pci0 dev 30 function 0 "Intel 82801BAM Hub-to-PCI" rev 0xa5 pci3 at ppb2 bus 3 pcib0 at pci0 dev 31 function 0 "Intel QS57 LPC" rev 0x05 ahci0 at pci0 dev 31 function 2 "Intel 3400 AHCI" rev 0x05: msi, AHCI 1.3 scsibus1 at ahci0: 32 targets sd0 at scsibus1 targ 0 lun 0: SCSI3 0/direct fixed t10.ATA_APPLE_SSD_TS256C_92CA4142K8HK sd0: 239372MB, 512 bytes/sector, 490234752 sectors, thin ichiic0 at pci0 dev 31 function 3 "Intel 3400 SMBus" rev 0x05: apic 2 int 18 iic0 at ichiic0 lisa0 at iic0 addr 0x1d: lis331dl spdmem0 at
LibreSSL OPENSSL_malloc... removal
Hello folks, The removal of OPENSSL_malloc/OPENSSL_free ... etc will cause a LOT of pain There is non négligeable number of applications which are strongly depending on this functionality, they use it for example to allocate SSL data structures in memory shared between multiple forked instances... One example of such application is Kamailio (sip server). Would you mind to reconsider removal of these routines?
Re: LibreSSL OPENSSL_malloc... removal
On 22 April 2014 08:49, Vadim Lebedev wrote: > Hello folks, > > The removal of OPENSSL_malloc/OPENSSL_free ... > etc will cause a LOT of pain There is non négligeable number of > applications which are strongly depending on this functionality, > they use it for example to allocate SSL data structures in memory shared > between multiple forked instances... > One example of such application is Kamailio (sip server). > > Would you mind to reconsider removal of these routines? > > > Yes we would mind. Ken
Re: LibreSSL OPENSSL_malloc... removal
> The removal of OPENSSL_malloc/OPENSSL_free ... > etc will cause a LOT of pain Which is why they are not removed. > There is non négligeable number of > applications which are strongly depending on this functionality, > they use it for example to allocate SSL data structures in memory shared > between multiple forked instances... > One example of such application is Kamailio (sip server). > > Would you mind to reconsider removal of these routines? They are not removed. Pay attention.
Question and regression test for strftime adn wcsftime
I see a lot of duplication between lib/libc/time/strftime.c and
lib/libc/time/wcsftime.c.
Since I would like to implement LC_TIME support, I would prefer to
change only in one place but before that I prepared a simple
regression test for strftime and wcsftime that is attached.
Checking NetBSD and FreeBSD sources I see they implemented wcsftime
by translating the wide character strings to multibyte, then calling
strftime and finally converting the result back to wide character:
http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libc/locale/wcsftime.c?rev=1.5&content-type=text/x-cvsweb-markup&only_with_tag=MAIN
https://github.com/freebsd/freebsd/blob/master/lib/libc/locale/wcsftime.c
The drawbacks I see are:
1. It is a little less efficient (because it has to translate before
calling strftime and translate again after)
2. If there is no memory or the conversions fail the function must
return 0 and set errno, and that is not specified in POSIX
http://pubs.opengroup.org/onlinepubs/009696799/functions/wcsftime.html
What way do you suggest me?
1. Implement like FreeBSD and NetBSD did
2. Change both functions? they use differente structures lc_time_T that at
least I would like to unify.
--
Dios, gracias por tu amor infinito.
--
Vladimir Támara Patiño. http://vtamara.pasosdeJesus.org/
http://www.pasosdejesus.org/dominio_publico_colombia.html
diff -ruN src56-orig/regress/lib/libc/time/Makefile
src/regress/lib/libc/time/Makefile
--- src56-orig/regress/lib/libc/time/Makefile Tue Jan 20 11:47:55 2004
+++ src/regress/lib/libc/time/Makefile Thu Apr 17 08:37:20 2014
@@ -1,5 +1,5 @@
# $OpenBSD: Makefile,v 1.1 2004/01/20 16:47:55 millert Exp $
-SUBDIR+=strptime
+SUBDIR+=ftime strptime
.include
diff -ruN src56-orig/regress/lib/libc/time/ftime/Makefile
src/regress/lib/libc/time/ftime/Makefile
--- src56-orig/regress/lib/libc/time/ftime/Makefile Wed Dec 31 19:00:00 1969
+++ src/regress/lib/libc/time/ftime/MakefileThu Apr 17 08:37:05 2014
@@ -0,0 +1,11 @@
+
+NOMAN=
+PROG=check_ftime
+
+CFLAGS=-g
+
+
+run-regress-check_ftime: ${PROG}
+ ./${PROG} >/dev/null
+
+.include
diff -ruN src56-orig/regress/lib/libc/time/ftime/check_ftime.c
src/regress/lib/libc/time/ftime/check_ftime.c
--- src56-orig/regress/lib/libc/time/ftime/check_ftime.cWed Dec 31
19:00:00 1969
+++ src/regress/lib/libc/time/ftime/check_ftime.c Thu Apr 17 11:46:19 2014
@@ -0,0 +1,268 @@
+/**
+ * Public domain according to Colombian Legislation.
+ * http://www.pasosdejesus.org/dominio_publico_colombia.html
+ * 2014. vtam...@pasosdejesus.org
+ *
+ * $OpenBSD$
+ */
+
+#include
+#include
+#include
+
+int bad;
+
+#define p(t) printf("%s:\t ",#t); \
+ if (t) { \
+ printf("\x1b[38;5;2mOK\x1b[0m\n"); \
+ } else { \
+ bad++; \
+ printf("\x1b[38;5;1mERROR\x1b[0m\n"); \
+ }
+
+
+void test_ftime_posix()
+{
+ char nom[256];
+ char col[512];
+ wchar_t wcol[512];
+ char *nl;
+ char *enc[]= { "ISO8859-1", "ISO8859-15", "UTF-8" };
+ struct lconv *p;
+ struct tm tl;
+ int i;
+ time_t ti;
+ long ts;
+ for(i = 0; i < sizeof(enc) / sizeof(char *) ; i++) {
+ snprintf(nom, sizeof(nom), "POSIX.%s", enc[i]);
+ printf("nom=%s\n", nom);
+ nl = setlocale(LC_ALL, nom);
+ printf("locale %s\n", nl);
+ p = localeconv();
+ ti = (time_t)139695; //Tue Apr 8 09:40:00 2014
+ gmtime_r(&ti, &tl) ;
+ /*p = strptime("", "", &tm); */
+ strftime(col, sizeof(col), "%A", &tl);
+ p(strcmp(col, "Tuesday") == 0);
+ wcsftime(wcol, sizeof(wcol), L"%A", &tl);
+ p(wcscmp(wcol, L"Tuesday") == 0);
+
+ strftime(col, sizeof(col), "%a", &tl);
+ p(strcmp(col, "Tue") == 0);
+ wcsftime(wcol, sizeof(wcol), L"%a", &tl);
+ p(wcscmp(wcol, L"Tue") == 0);
+
+ strftime(col, sizeof(col), "%B", &tl);
+ p(strcmp(col, "April") == 0);
+ wcsftime(wcol, sizeof(wcol), L"%B", &tl);
+ p(wcscmp(wcol, L"April") == 0);
+
+ strftime(col, sizeof(col), "%b", &tl);
+ p(strcmp(col, "Apr") == 0);
+ wcsftime(wcol, sizeof(wcol), L"%b", &tl);
+ p(wcscmp(wcol, L"Apr") == 0);
+
+ strftime(col, sizeof(col), "%C", &tl);
+ p(strcmp(col, "20") == 0);
+ wcsftime(wcol, sizeof(wcol), L"%C", &tl);
+ p(wcscmp(wcol, L"20") == 0);
+
+ strftime(col, sizeof(col), "%c", &tl);
+ printf("%s\n", col);
+ p(strcmp(col, "Tue Apr 8 09:40:00 2014") == 0);
+ wcsftime(wcol, sizeof(wcol), L"%c", &tl);
+ p(wcscmp(wcol, L"Tue Apr 8 09:40:00 2014") == 0);
+
+ strftime(col, sizeof(col), "%D", &tl);
+
Re: LibreSSL OPENSSL_malloc... removal
Theo de Raadt cvs.openbsd.org> writes: > > > The removal of OPENSSL_malloc/OPENSSL_free ... > > etc will cause a LOT of pain > > Which is why they are not removed. > > > There is non négligeable number of > > applications which are strongly depending on this functionality, > > they use it for example to allocate SSL data structures in memory shared > > between multiple forked instances... > > One example of such application is Kamailio (sip server). > > > > Would you mind to reconsider removal of these routines? > > They are not removed. Pay attention.y thios > > I was alarmed by this: http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libssl/src/ssl/s3_clnt.c.diff? r1=1.34;r2=1.35;f=h Maybe i do misread something else but it seems like calls to OPENSSL_malloc/free routines are being replaced by malloc/free etc.. Would you mind to explain what i've been missing? Thanks Vadim
Re: LibreSSL OPENSSL_malloc... removal
Hi, Vadim Lebedev wrote on Tue, Apr 22, 2014 at 01:17:16PM +: > Theo de Raadt cvs.openbsd.org> writes: >> Vadim Lebedev wrote: >>> The removal of OPENSSL_malloc/OPENSSL_free ... >>> etc will cause a LOT of pain >> Which is why they are not removed. > I was alarmed by this: > > http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libssl/src/ssl/ > s3_clnt.c.diff?r1=1.34;r2=1.35;f=h > > Maybe i do misread something else but it seems like calls > to OPENSSL_malloc/free routines are being replaced by malloc/free etc.. That is true. > Would you mind to explain what i've been missing? To stop using functions internally, inside the same library, is not the same as deleting them from the library interface for external use. By the way, this discussion would fit better on misc@ than on tech@, it isn't about technical suggestions, but about basic understanding, so consider moving it there if you think you still must reply. Yours, Ingo
Re: LibreSSL OPENSSL_malloc... removal
On Tue, Apr 22, 2014 at 13:17, Vadim Lebedev wrote: > I was alarmed by this: > > http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libssl/src/ssl/s3_clnt.c.diff? > r1=1.34;r2=1.35;f=h > > Maybe i do misread something else but it seems like calls > to OPENSSL_malloc/free routines are being replaced by malloc/free etc.. > > Would you mind to explain what i've been missing? You are correct. The interface remains for applications that use it, but it's no longer possible to change its behavior. We believe overriding these functions to allow sharing memory like you're describing is an inherently insecure construction, and we won't allow it.
iked + isakmpd on the same machine
It happened! A remote peer *requires* IKEv2 - and I've to do that on a machine running isakmpd with somewhat 25+ IKEv1 peers. First hurdle: I cannot bind iked to a certain (carp) IP-address. Mad workaround: start isakmpd (with Listen-on) first. Second hurdle: iked loads "its" SAs and eventually does this by creating a new empty SADB, effectivly killing all the SAs isakmpd loaded into the kernel before? Is there a diff sleeping out there for tackling the first hurdle? For the second one, I've to refrain from testing this in live further more. First to reconstruct my Frankenstein-Lab. Cheers for any thoughts beside "mad, bro?" :-)
Re: iked + isakmpd on the same machine
On 22 April 2014 17:13, Philipp wrote: > It happened! A remote peer *requires* IKEv2 - and I've to do that on a > machine running isakmpd with somewhat 25+ IKEv1 peers. > > First hurdle: I cannot bind iked to a certain (carp) IP-address. Mad > workaround: start isakmpd (with Listen-on) first. > Second hurdle: iked loads "its" SAs and eventually does this by creating a > new empty SADB, effectivly killing all > the SAs isakmpd loaded into the kernel before? > > Is there a diff sleeping out there for tackling the first hurdle? > > For the second one, I've to refrain from testing this in live further more. > First to reconstruct my Frankenstein-Lab. > > Cheers for any thoughts beside "mad, bro?" :-) > more like it's not supported and is not supposed to work. it's like running nginx and apache at the same time but worse since there are kernel tentacles involved as well (as you might have figured out already) that will likely prevent you from doing that on the same box but different ip addresses. cheers, mike
typo security.8
Hi tech, it's Trojan horse not Trojan horsed, right? Fritjof Index: security.8 === RCS file: /cvs/src/share/man/man8/security.8,v retrieving revision 1.23 diff -u -p -r1.23 security.8 --- security.8 20 Apr 2014 22:15:49 - 1.23 +++ security.8 22 Apr 2014 16:25:09 - @@ -84,7 +84,7 @@ to protect the programs in .Pp .Sy Note: These checks do not provide complete protection against -Trojan horsed binaries, as +Trojan horse binaries, as the miscreant can modify the tree specification to match the replaced binary. For details on really protecting yourself against modified binaries, see .Xr mtree 8 .
Re: typo security.8
* Fritjof Bornebusch [2014-04-22 18:29]: > it's Trojan horse not Trojan horsed, right? yup. a trojan horse. the binary has been trojan horsed. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
Re: typo security.8
On 22 Apr 2014, at 18:32, Henning Brauer wrote: > the binary has been trojan horsed. Not sure if urban dictionary should be a terminology pool for manual pages. Also, there's clearly a hyphen missing: ``trojan-horsed''. No capital T obviously since the term is common computer language and not a book, a city, etc. ;) Cheers, Franco
Re: typo security.8
On Tue, Apr 22, 2014 at 06:32:12PM +0200, Henning Brauer wrote: > * Fritjof Bornebusch [2014-04-22 18:29]: > > it's Trojan horse not Trojan horsed, right? > > yup. > a trojan horse. > the binary has been trojan horsed. As Henning notes, it was gramatically correct as written (if you are willing to accept "trojan horse" as a neologisticized verb), although potentially confusing to non-native speakers. I wonder if it might be better style to say smth like: a trojan-infested binary or ?
Re: typo security.8
On Tue, Apr 22, 2014 at 18:28, Fritjof Bornebusch wrote: > Hi tech, > > it's Trojan horse not Trojan horsed, right? > > Fritjof > > Index: security.8 > === > RCS file: /cvs/src/share/man/man8/security.8,v > retrieving revision 1.23 > diff -u -p -r1.23 security.8 > --- security.8 20 Apr 2014 22:15:49 - 1.23 > +++ security.8 22 Apr 2014 16:25:09 - > @@ -84,7 +84,7 @@ to protect the programs in > .Pp > .Sy Note: > These checks do not provide complete protection against > -Trojan horsed binaries, as > +Trojan horse binaries, as Yes. It's like a cheese sandwich. We don't call it a cheesed sandwich.
Re: reviewing OpenSSL's lib/libssl/src/crypto/asn1
My bad Dirk - you're right with that one.
I'll take a look at this when I get home, and either apply your fix or
disentangle this in a hopefully more obvious way.
On Mon, Apr 21, 2014 at 1:53 PM, Dirk Engling wrote:
> On 21.04.14 19:01, Bob Beck wrote:
>
>> Not quite, because now you avoid the potential double free and instead leak
>> ret itself because of how ASN1_STRING_free works.. You need to
>> do this slightly differently.
>
> I disagree:
>
> err:
> if ((ret != NULL) && ((a == NULL) || (*a != ret)))
> ASN1_STRING_free(ret);
>
> clearly, ret is not NULL and in order to have a ASN1_STRING we need to
> free, the if ((a == NULL) || ((*a) == NULL)) on top must have triggered.
> So indeed ASN1_STRING_free is called with
>
> The ASN1_STRING_free function itself is rather straight forward:
>
> void
> ASN1_STRING_free(ASN1_STRING *a)
> {
> if (a == NULL)
> return;
> if (a->data && !(a->flags & ASN1_STRING_FLAG_NDEF))
> free(a->data);
> free(a);
> }
>
> IOW: it only does abort early and thus not free() ret, if ret was NULL,
> which is not the case once we get here. It will then free() ret->data if
> it is not NULL (the double free case I wanted to solve) and then goes on
> to free() ret.
>
> Maybe I have missed the magic where a NULL data field in ASN1_STRING
> makes ASN1_STRING_free not free the object, but I can't find that, at
> least not in my source tree.
>
> erdgeist
>
Re: reviewing OpenSSL's lib/libssl/src/crypto/asn1
On 22.04.14 19:16, Bob Beck wrote: > I'll take a look at this when I get home, and either apply your fix or > disentangle this in a hopefully more obvious way. How shall I proceed with the other fixes? Just bundle them as diffs against the current revision an put them on the list as new threads? What about some other comments on the code? I found the use of (long)time() in timeout checks and wonder if it is more appropriate to use clock_gettime with CLOCK_MONOTONIC when available. The wall clock is expected to drift and even jump backwards in presence of an ntpd. erdgeist
Re: reviewing OpenSSL's lib/libssl/src/crypto/asn1
Post diffs one per message per "thing you're trying to do" - example "fix leak in foo.c" - etc. You may have slow replies for a few days, people are travelling On Tue, Apr 22, 2014 at 12:12 PM, Dirk Engling wrote: > On 22.04.14 19:16, Bob Beck wrote: > >> I'll take a look at this when I get home, and either apply your fix or >> disentangle this in a hopefully more obvious way. > > How shall I proceed with the other fixes? Just bundle them as diffs > against the current revision an put them on the list as new threads? > > What about some other comments on the code? I found the use of > (long)time() in timeout checks and wonder if it is more appropriate to > use clock_gettime with CLOCK_MONOTONIC when available. The wall clock is > expected to drift and even jump backwards in presence of an ntpd. > > erdgeist >
[PATCH] usr.bin/sdiff/sdiff.c prompt bikeshedding
Personally, when sysmerge asks if I'd like to merge something I'd
prefer a little breathing room at the prompt. (Will this break
anything?)
Index: usr.bin/sdiff//sdiff.c
===
RCS file: /work/cvsroot/src/usr.bin/sdiff/sdiff.c,v
retrieving revision 1.30
diff -p -u -r1.30 sdiff.c
--- usr.bin/sdiff//sdiff.c 26 Nov 2013 21:08:12 - 1.30
+++ usr.bin/sdiff//sdiff.c 22 Apr 2014 18:27:38 -
@@ -421,6 +421,7 @@ prompt(const char *s1, const char *s2)
/* Print command prompt. */
putchar('%');
+ putchar(' ');
/* Get user input. */
for (; (cmd = xfgets(stdin)); free(cmd)) {
@@ -474,6 +475,7 @@ USAGE:
int_usage();
PROMPT:
putchar('%');
+ putchar(' ');
/* Prompt user again. */
continue;
Re: [PATCH] usr.bin/sdiff/sdiff.c prompt bikeshedding
Sorry, my webmail client ate the whitespace. mutt is a casualty of the
Valhalla rampage (whines about permissions on /dev/arandom) and kerberosV
removal. I'll rebuild mutt from ports and resubmit.
-Original Message-
From: "Kent R. Spillner"
Sent: Tuesday, April 22, 2014 13:29
To: tech@openbsd.org
Subject: [PATCH] usr.bin/sdiff/sdiff.c prompt bikeshedding
Personally, when sysmerge asks if I'd like to merge something I'd
prefer a little breathing room at the prompt. (Will this break
anything?)
Index: usr.bin/sdiff//sdiff.c
===
RCS file: /work/cvsroot/src/usr.bin/sdiff/sdiff.c,v
retrieving revision 1.30
diff -p -u -r1.30 sdiff.c
--- usr.bin/sdiff//sdiff.c 26 Nov 2013 21:08:12 - 1.30
+++ usr.bin/sdiff//sdiff.c 22 Apr 2014 18:27:38 -
@@ -421,6 +421,7 @@ prompt(const char *s1, const char *s2)
/* Print command prompt. */
putchar('%');
+ putchar(' ');
/* Get user input. */
for (; (cmd = xfgets(stdin)); free(cmd)) {
@@ -474,6 +475,7 @@ USAGE:
int_usage();
PROMPT:
putchar('%');
+ putchar(' ');
/* Prompt user again. */
continue;
[PATCH] www/faq/current.html tweak
rm: /usr/libdata/perl5/site_perl/amd64-openbsd/kerberosV: is a directory
Index: faq/current.html
===
RCS file: /work/cvsroot/www/faq/current.html,v
retrieving revision 1.502
diff -p -u -r1.502 current.html
--- faq/current.html22 Apr 2014 15:19:10 - 1.502
+++ faq/current.html22 Apr 2014 17:21:47 -
@@ -965,7 +965,7 @@ the system.
rm -f /usr/lib/libkrb5{,_p}.*
rm -f /usr/lib/libroken{,_p}.*
rm -f /usr/lib/libwind{,_p}.*
- rm -f /usr/libdata/perl5/site_perl/amd64-openbsd/kerberosV/
+ rm -rf /usr/libdata/perl5/site_perl/amd64-openbsd/kerberosV/
rm -f /usr/libexec/auth/login_krb5{,-or-pwd}
rm -f /usr/libexec/hprop{,d}
rm -f /usr/libexec/ipropd-{master,slave}
Re: [PATCH] www/faq/current.html tweak
Committed, thanks. -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: [patch ping.c] replace malloc & memset with calloc
Thanks Florian & team.
Please review the following diff.
Index: ping.c
===
RCS file: /cvs/src/sbin/ping/ping.c,v
retrieving revision 1.100
diff -u -p -u -r1.100 ping.c
--- ping.c 24 Mar 2014 11:11:49 - 1.100
+++ ping.c 22 Apr 2014 19:00:28 -
@@ -72,6 +72,7 @@
#include
#include
#include
+#include
struct tv32 {
u_int tv32_sec;
@@ -175,7 +176,7 @@ void usage(void);
int
main(int argc, char *argv[])
{
- struct timeval timeout;
+ int timeout;
struct hostent *hp;
struct sockaddr_in *to;
struct in_addr saddr;
@@ -187,8 +188,7 @@ main(int argc, char *argv[])
#endif
socklen_t maxsizelen;
const char *errstr;
- fd_set *fdmaskp;
- size_t fdmasks;
+ struct pollfd fdmaskp[1];
uid_t uid;
u_int rtableid;
@@ -507,10 +507,6 @@ main(int argc, char *argv[])
if ((options & F_FLOOD) == 0)
catcher(0); /* start things going */
- fdmasks = howmany(s+1, NFDBITS) * sizeof(fd_mask);
- if ((fdmaskp = (fd_set *)malloc(fdmasks)) == NULL)
- err(1, "malloc");
-
for (;;) {
struct sockaddr_in from;
sigset_t omask, nmask;
@@ -519,13 +515,19 @@ main(int argc, char *argv[])
if (options & F_FLOOD) {
pinger();
- timeout.tv_sec = 0;
- timeout.tv_usec = 1;
- memset(fdmaskp, 0, fdmasks);
- FD_SET(s, fdmaskp);
- if (select(s + 1, (fd_set *)fdmaskp, (fd_set *)NULL,
- (fd_set *)NULL, &timeout) < 1)
- continue;
+ timeout = 10;
+ fdmaskp[0].fd = s;
+ fdmaskp[0].events = POLLIN;
+ cc = poll(fdmaskp, 1, timeout);
+
+ if (cc < 0) {
+ if (errno != EINTR) {
+ warn("poll");
+ sleep(1);
+ }
+ continue;
+ } else if (cc == 0)
+ continue;
}
fromlen = sizeof(from);
if ((cc = recvfrom(s, packet, packlen, 0,
@@ -543,7 +545,6 @@ main(int argc, char *argv[])
if (npackets && nreceived >= npackets)
break;
}
- free(fdmaskp);
finish(0);
/* NOTREACHED */
exit(0);/* Make the compiler happy */
Quoting Florian Obser :
Please switch it to poll(2) like ping6(8) is doing, there by side
stepping the whole issue.
On Tue, Apr 22, 2014 at 09:33:50AM +0200, Otto Moerbeek wrote:
On Tue, Apr 22, 2014 at 02:57:54AM -0400, pe...@petermalone.org wrote:
> Sure - I should have spotted that.
Still not there. Please use the fact that calloc can multiply, you get
an overflow check for free.
-Otto
>
> Index: ping.c
> ===
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.100
> diff -u -r1.100 ping.c
> --- ping.c 24 Mar 2014 11:11:49 - 1.100
> +++ ping.c 22 Apr 2014 06:55:03 -
> @@ -188,7 +188,6 @@
> socklen_t maxsizelen;
> const char *errstr;
> fd_set *fdmaskp;
> - size_t fdmasks;
> uid_t uid;
> u_int rtableid;
>
> @@ -507,9 +506,8 @@
> if ((options & F_FLOOD) == 0)
> catcher(0); /* start things going */
>
> - fdmasks = howmany(s+1, NFDBITS) * sizeof(fd_mask);
> - if ((fdmaskp = (fd_set *)malloc(fdmasks)) == NULL)
> - err(1, "malloc");
> + if ((fdmaskp = calloc(1, howmany(s+1, NFDBITS) * sizeof(fd_mask)))
> == NULL)
> + err(1, "calloc");
>
> for (;;) {
> struct sockaddr_in from;
> @@ -521,7 +519,6 @@
> pinger();
> timeout.tv_sec = 0;
> timeout.tv_usec = 1;
> - memset(fdmaskp, 0, fdmasks);
> FD_SET(s, fdmaskp);
> if (select(s + 1, (fd_set *)fdmaskp,
(fd_set *)NULL,
> (fd_set *)NULL, &timeout) < 1)
>
>
> Quoting Otto Moerbeek :
>
> >On Tue, Apr 22, 2014 at 12:45:25AM -0400, Peter Malone wrote:
> >
> >>Hi,
> >>
> >>malloc & memset can be replaced with calloc in ping.c. Please
see below for
> >>patch details:
> >
> >Better rework this to get rid of fdmasks.
> >
> > -Otto
> >
> >>
> >>Index: ping.c
> >>===
> >>RCS file: /cvs/src/sbin/ping/ping.c,v
> >>retrieving revision 1.100
> >>diff -u -p -u -r1.100 ping.c
> >>--- ping.c24 Mar 2014 1
[patch dmesg.c] replace malloc & memset with calloc
Hi, Another cleanup of malloc & memset with calloc. This time with dmesg. Index: dmesg.c === RCS file: /cvs/src/sbin/dmesg/dmesg.c,v retrieving revision 1.22 diff -u -p -u -r1.22 dmesg.c --- dmesg.c 2 Jul 2010 22:02:06 - 1.22 +++ dmesg.c 22 Apr 2014 19:33:36 - @@ -95,11 +95,10 @@ main(int argc, char *argv[]) err(1, "sysctl: KERN_MSGBUFSIZE"); msgbufsize += sizeof(struct msgbuf) - 1; - bufdata = malloc(msgbufsize); + bufdata = calloc(1, msgbufsize); if (bufdata == NULL) errx(1, "couldn't allocate space for buffer data"); - memset(bufdata, 0, msgbufsize); mib[1] = KERN_MSGBUF; len = msgbufsize; if (sysctl(mib, 2, bufdata, &len, NULL, 0))
[Patch] ftp.c "never read" variables
Hi tech,
there are some "never read" variables in ftp.c.
The local variable "error" is set but there is either a
goto bad;
bad:
(void)close(data), data = -1;
if (tmpno)
sendport = 1;
return (1);
and the variable is not going to read here.
or the variable is just set but never read - without a goto statement.
Am I right, or are there other reasons why the error variable is set but not
used?
Fritjof
Index: ftp.c
===
RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
retrieving revision 1.84
diff -u -p -r1.84 ftp.c
--- ftp.c 20 Dec 2013 13:44:51 - 1.84
+++ ftp.c 22 Apr 2014 19:40:33 -
@@ -1408,13 +1408,11 @@ reinit:
if (data_addr.su_family != AF_INET) {
fputs(
"Passive mode AF mismatch. Shouldn't happen!\n", ttyout);
- error = 1;
goto bad;
}
if (code / 10 == 22 && code != 227) {
fputs("wrong server: return code must be 227\n",
ttyout);
- error = 1;
goto bad;
}
error = sscanf(pasv, "%u,%u,%u,%u,%u,%u",
@@ -1423,10 +1421,8 @@ reinit:
if (error != 6) {
fputs(
"Passive mode address scan failure. Shouldn't happen!\n", ttyout);
- error = 1;
goto bad;
}
- error = 0;
memset(&data_addr, 0, sizeof(data_addr));
data_addr.su_family = AF_INET;
data_addr.su_len = sizeof(struct sockaddr_in);
@@ -1437,7 +1433,6 @@ reinit:
if (code / 10 == 22 && code != 228) {
fputs("wrong server: return code must be 228\n",
ttyout);
- error = 1;
goto bad;
}
switch (data_addr.su_family) {
@@ -1450,17 +1445,14 @@ reinit:
if (error != 9) {
fputs(
"Passive mode address scan failure. Shouldn't happen!\n", ttyout);
- error = 1;
goto bad;
}
if (af != 4 || hal != 4 || pal != 2) {
fputs(
"Passive mode AF mismatch. Shouldn't happen!\n", ttyout);
- error = 1;
goto bad;
}
- error = 0;
memset(&data_addr, 0, sizeof(data_addr));
data_addr.su_family = AF_INET;
data_addr.su_len = sizeof(struct sockaddr_in);
@@ -1481,17 +1473,14 @@ reinit:
if (error != 21) {
fputs(
"Passive mode address scan failure. Shouldn't happen!\n", ttyout);
- error = 1;
goto bad;
}
if (af != 6 || hal != 16 || pal != 2) {
fputs(
"Passive mode AF mismatch. Shouldn't happen!\n", ttyout);
- error = 1;
goto bad;
}
- error = 0;
memset(&data_addr, 0, sizeof(data_addr));
data_addr.su_family = AF_INET6;
data_addr.su_len = sizeof(struct sockaddr_in6);
@@ -1515,20 +1504,17 @@ reinit:
if (code / 10 == 22 && code != 229) {
fputs("wrong server: return code must be 229\n",
ttyout);
- error = 1;
goto bad;
}
if (sscanf(pasv, "%c%c%c%d%c", &delim[0],
&delim[1], &delim[2], &port[1],
&delim[3]) != 5) {
fputs("parse error!\n", ttyout);
- error = 1;
goto bad;
}
if (delim[0] != delim[1] || delim[0] != delim[2]
|| delim[0] != delim[3]) {
fputs("pa
[patch mountd.c] replace malloc & memset with calloc
Hi,
Another malloc & memset to calloc cleanup. This time in mountd.c
Index: mountd.c
===
RCS file: /cvs/src/sbin/mountd/mountd.c,v
retrieving revision 1.73
diff -u -p -u -r1.73 mountd.c
--- mountd.c24 Mar 2014 00:19:48 - 1.73
+++ mountd.c22 Apr 2014 19:47:55 -
@@ -1057,10 +1057,9 @@ get_exp(void)
{
struct exportlist *ep;
- ep = (struct exportlist *)malloc(sizeof (struct exportlist));
+ ep = calloc(1, sizeof (struct exportlist));
if (ep == NULL)
out_of_mem();
- memset(ep, 0, sizeof(struct exportlist));
return (ep);
}
@@ -1072,10 +1071,9 @@ get_grp(void)
{
struct grouplist *gp;
- gp = (struct grouplist *)malloc(sizeof (struct grouplist));
+ gp = calloc(1, sizeof (struct grouplist));
if (gp == NULL)
out_of_mem();
- memset(gp, 0, sizeof(struct grouplist));
return (gp);
}
Re: 12 seconds delay when starting X.org
>Try to disable pms: > ># config -e -f /bsd >UKC>disable pms > >And see if you can reproduce the problem. I can, after disabling pms device, X.org starts just fine. Great. Could you compile a kernel with the diff below applied and post its dmesg after starting Xorg. I'm guessing that the delay comes from the fact that we are re-probing for the touchpad protocol, but having this debug output would help. Thanks! I did, and this is what it looks like now: OpenBSD 5.5-current (GENERIC.MP) #0: Tue Apr 22 20:10:57 EEST 2014 xxx@xxx:/usr/src/sys/arch/amd64/compile/GENERIC.MP RTC BIOS diagnostic error 30 real mem = 4133875712 (3942MB) avail mem = 4015108096 (3829MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.6 @ 0xf2450 (66 entries) bios0: vendor Dell Inc. version "A14" date 12/05/2013 bios0: Dell Inc. Latitude E4310 acpi0 at bios0: rev 2 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP APIC TCPA MCFG HPET BOOT SLIC SSDT acpi0: wakeup devices AGP_(S4) P0P1(S4) UAR1(S3) HDEF(S4) PXSX(S4) RP01(S4) PXSX(S4) RP02(S4) PXSX(S4) RP03(S4) PXSX(S4) RP04(S4) PXSX(S4) RP05(S4) PXSX(S4) RP07(S4) [...] acpitimer0 at acpi0: 3579545 Hz, 24 bits acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Core(TM) i5 CPU M 560 @ 2.67GHz, 2926.56 MHz cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,POPCNT,AES,NXE,LONG,LAHF,PERF,ITSC cpu0: 256KB 64b/line 8-way L2 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges cpu0: apic clock running at 133MHz cpu0: mwait min=64, max=64, C-substates=0.2.1.1.0, IBE cpu1 at mainbus0: apid 4 (application processor) cpu1: Intel(R) Core(TM) i5 CPU M 560 @ 2.67GHz, 2926.00 MHz cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,POPCNT,AES,NXE,LONG,LAHF,PERF,ITSC cpu1: 256KB 64b/line 8-way L2 cache cpu1: smt 0, core 2, package 0 ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 24 pins acpimcfg0 at acpi0 addr 0xf800, bus 0-63 acpihpet0 at acpi0: 14318179 Hz acpiprt0 at acpi0: bus 0 (PCI0) acpiprt1 at acpi0: bus -1 (AGP_) acpiprt2 at acpi0: bus 3 (P0P1) acpiprt3 at acpi0: bus 1 (RP01) acpiprt4 at acpi0: bus -1 (RP02) acpiprt5 at acpi0: bus 2 (RP03) acpiprt6 at acpi0: bus -1 (RP04) acpiprt7 at acpi0: bus -1 (RP05) acpiprt8 at acpi0: bus -1 (RP07) acpiprt9 at acpi0: bus -1 (RP08) acpiprt10 at acpi0: bus -1 (PEG3) acpiprt11 at acpi0: bus -1 (PEG5) acpiec0 at acpi0 acpicpu0 at acpi0: C3, C1, PSS acpicpu1 at acpi0: C3, C1, PSS acpibtn0 at acpi0: LID_ acpibtn1 at acpi0: PBTN acpibtn2 at acpi0: SBTN acpiac0 at acpi0: AC unit online acpibat0 at acpi0: BAT0 model "DELL PXD3611" serial 36834 type LION oem "Samsung SDI" acpibat1 at acpi0: BAT1 not present acpivideo0 at acpi0: VID_ acpivout0 at acpivideo0: LCD_ cpu0: Enhanced SpeedStep 2926 MHz: speeds: 2667, 2666, 2533, 2399, 2266, 2133, 1999, 1866, 1733, 1599, 1466, 1333, 1199 MHz pci0 at mainbus0 bus 0 pchb0 at pci0 dev 0 function 0 "Intel Core Host" rev 0x02 vga1 at pci0 dev 2 function 0 "Intel HD Graphics" rev 0x02 intagp0 at vga1 agp0 at intagp0: aperture at 0xe000, size 0x1000 inteldrm0 at vga1 drm0 at inteldrm0 inteldrm0: 1366x768 wsdisplay0 at vga1 mux 1: console (std, vt100 emulation) wsdisplay0: screen 1-5 added (std, vt100 emulation) em0 at pci0 dev 25 function 0 "Intel 82577LM" rev 0x05: msi, address 5c:26:0a:3e:bd:bc ehci0 at pci0 dev 26 function 0 "Intel 3400 USB" rev 0x05: apic 2 int 16 usb0 at ehci0: USB revision 2.0 uhub0 at usb0 "Intel EHCI root hub" rev 2.00/1.00 addr 1 azalia0 at pci0 dev 27 function 0 "Intel QS57 HD Audio" rev 0x05: msi azalia0: codecs: IDT 92HD81B1X, Intel/0x2804, using IDT 92HD81B1X audio0 at azalia0 ppb0 at pci0 dev 28 function 0 "Intel 3400 PCIE" rev 0x05: msi pci1 at ppb0 bus 1 ppb1 at pci0 dev 28 function 2 "Intel 3400 PCIE" rev 0x05: msi pci2 at ppb1 bus 2 sdhc0 at pci2 dev 0 function 0 "Ricoh 5U822 SD/MMC" rev 0x01: apic 2 int 18 sdmmc0 at sdhc0 ehci1 at pci0 dev 29 function 0 "Intel 3400 USB" rev 0x05: apic 2 int 17 usb1 at ehci1: USB revision 2.0 uhub1 at usb1 "Intel EHCI root hub" rev 2.00/1.00 addr 1 ppb2 at pci0 dev 30 function 0 "Intel 82801BAM Hub-to-PCI" rev 0xa5 pci3 at ppb2 bus 3 pcib0 at pci0 dev 31 function 0 "Intel QS57 LPC" rev 0x05 ahci0 at pci0 dev 31 function 2 "Intel 3400 AHCI" rev 0x05: msi, AHCI 1.3 scsibus1 at ahci0: 32 targets sd0 at scsibus1 targ 0 lun 0: SCSI3 0/direct fixed t10.ATA_APPLE_SSD_TS256C_92CA4142K8HK sd0: 239372MB, 512 bytes/sector, 490234752 sectors, thin ichiic0 at pci0 dev 31 function 3 "Intel 3400 SMBus" rev 0x05: apic 2 int 18 iic0 at ichiic0 lisa0
[patch init.c] replace malloc & memset with calloc
Hi,
Another replacement of malloc & memset with calloc. This time in init.c.
Also added a check as non existed prior to this.
Index: init.c
===
RCS file: /cvs/src/sbin/init/init.c,v
retrieving revision 1.49
diff -u -p -u -r1.49 init.c
--- init.c 3 Jan 2014 22:29:00 - 1.49
+++ init.c 22 Apr 2014 19:56:27 -
@@ -860,8 +860,11 @@ new_session(session_t *sprev, int sessio
typ->ty_getty == 0)
return (0);
- sp = (session_t *) malloc(sizeof (session_t));
- memset(sp, 0, sizeof *sp);
+ sp = calloc(1, sizeof (session_t));
+ if (sp == NULL) {
+ err(1, "calloc");
+ return (0);
+ }
sp->se_flags = SE_PRESENT;
sp->se_index = session_index;
[patch bioctl.c] replace malloc & memseet with calloc
Hi,
Another replacement of malloc & memset with calloc. This time in bioctl.c.
Index: bioctl.c
===
RCS file: /cvs/src/sbin/bioctl/bioctl.c,v
retrieving revision 1.119
diff -u -p -u -r1.119 bioctl.c
--- bioctl.c18 Jan 2014 09:11:12 - 1.119
+++ bioctl.c22 Apr 2014 20:02:39 -
@@ -836,10 +836,9 @@ bio_createraid(u_int16_t level, char *de
} else
#endif /* AOE */
{
- dt = (dev_t *)malloc(BIOC_CRMAXLEN);
+ dt = calloc(1, BIOC_CRMAXLEN);
if (!dt)
err(1, "not enough memory for dev_t list");
- memset(dt, 0, BIOC_CRMAXLEN);
no_dev = bio_parse_devlist(dev_list, dt);
}
Re: [patch init.c] replace malloc & memset with calloc
On Tue, Apr 22, 2014 at 03:58:23PM -0400, pe...@petermalone.org wrote:
> Hi,
>
> Another replacement of malloc & memset with calloc. This time in init.c.
> Also added a check as non existed prior to this.
>
> Index: init.c
> ===
> RCS file: /cvs/src/sbin/init/init.c,v
> retrieving revision 1.49
> diff -u -p -u -r1.49 init.c
> --- init.c 3 Jan 2014 22:29:00 - 1.49
> +++ init.c 22 Apr 2014 19:56:27 -
> @@ -860,8 +860,11 @@ new_session(session_t *sprev, int sessio
> typ->ty_getty == 0)
> return (0);
>
> - sp = (session_t *) malloc(sizeof (session_t));
> - memset(sp, 0, sizeof *sp);
> + sp = calloc(1, sizeof (session_t));
> + if (sp == NULL) {
> + err(1, "calloc");
The err(), verr(), errx(), and verrx() functions do not return, but exit
with the value of the argument eval.
A library generally shouldn't exit.
> + return (0);
> + }
>
> sp->se_flags = SE_PRESENT;
> sp->se_index = session_index;
Re: [patch init.c] replace malloc & memset with calloc
On Tue, Apr 22, 2014 at 10:07:10PM +0200, Stefan Sperling wrote:
> On Tue, Apr 22, 2014 at 03:58:23PM -0400, pe...@petermalone.org wrote:
> > Hi,
> >
> > Another replacement of malloc & memset with calloc. This time in init.c.
> > Also added a check as non existed prior to this.
> >
> > Index: init.c
> > ===
> > RCS file: /cvs/src/sbin/init/init.c,v
> > retrieving revision 1.49
> > diff -u -p -u -r1.49 init.c
> > --- init.c 3 Jan 2014 22:29:00 - 1.49
> > +++ init.c 22 Apr 2014 19:56:27 -
> > @@ -860,8 +860,11 @@ new_session(session_t *sprev, int sessio
> > typ->ty_getty == 0)
> > return (0);
> >
> > - sp = (session_t *) malloc(sizeof (session_t));
> > - memset(sp, 0, sizeof *sp);
> > + sp = calloc(1, sizeof (session_t));
> > + if (sp == NULL) {
> > + err(1, "calloc");
>
> The err(), verr(), errx(), and verrx() functions do not return, but exit
> with the value of the argument eval.
>
> A library generally shouldn't exit.
Ah, sorry :) Somehow I was assuming this was a libssl diff.
Re: [patch init.c] replace malloc & memset with calloc
No worries. :)
Quoting Stefan Sperling :
On Tue, Apr 22, 2014 at 10:07:10PM +0200, Stefan Sperling wrote:
On Tue, Apr 22, 2014 at 03:58:23PM -0400, pe...@petermalone.org wrote:
> Hi,
>
> Another replacement of malloc & memset with calloc. This time in init.c.
> Also added a check as non existed prior to this.
>
> Index: init.c
> ===
> RCS file: /cvs/src/sbin/init/init.c,v
> retrieving revision 1.49
> diff -u -p -u -r1.49 init.c
> --- init.c 3 Jan 2014 22:29:00 - 1.49
> +++ init.c 22 Apr 2014 19:56:27 -
> @@ -860,8 +860,11 @@ new_session(session_t *sprev, int sessio
> typ->ty_getty == 0)
> return (0);
>
> - sp = (session_t *) malloc(sizeof (session_t));
> - memset(sp, 0, sizeof *sp);
> + sp = calloc(1, sizeof (session_t));
> + if (sp == NULL) {
> + err(1, "calloc");
The err(), verr(), errx(), and verrx() functions do not
return, but exit
with the value of the argument eval.
A library generally shouldn't exit.
Ah, sorry :) Somehow I was assuming this was a libssl diff.
[patch dhclient/clparse.c] replace malloc & memset with calloc
Hi,
Another replacement of malloc & memset with calloc. This time in
dhclient/clparse.c
RCS file: /cvs/src/sbin/dhclient/clparse.c,v
retrieving revision 1.83
diff -u -p -u -r1.83 clparse.c
--- clparse.c 25 Jan 2014 05:21:23 - 1.83
+++ clparse.c 22 Apr 2014 20:16:36 -
@@ -454,10 +454,9 @@ parse_client_lease_statement(FILE *cfile
return;
}
- lease = malloc(sizeof(struct client_lease));
+ lease = calloc(1, sizeof (struct client_lease));
if (!lease)
error("no memory for lease.");
- memset(lease, 0, sizeof(*lease));
lease->is_static = is_static;
do {
Re: [patch mountd.c] replace malloc & memset with calloc
On Tue, Apr 22, 2014 at 15:49, pe...@petermalone.org wrote: > Hi, > > Another malloc & memset to calloc cleanup. This time in mountd.c Can you be careful about whitespace? Your diffs have spaces instead of tabs and don't apply. The ones you've sent I can hand apply, but for future diffs please work out something that doesn't require me to do any typing. :)
Re: [patch mountd.c] replace malloc & memset with calloc
Note you can often have this problem if you cut and paste your diff into a mail message Normally best to use something that allows you to just include the diff from a file inline. To check it, mail *yourself* the diff. If you can save the email message raw and apply the diff just by running patch with the email as input, you're good. On Tue, Apr 22, 2014 at 2:23 PM, Ted Unangst wrote: > On Tue, Apr 22, 2014 at 15:49, pe...@petermalone.org wrote: >> Hi, >> >> Another malloc & memset to calloc cleanup. This time in mountd.c > > Can you be careful about whitespace? Your diffs have spaces instead of > tabs and don't apply. > > The ones you've sent I can hand apply, but for future diffs please > work out something that doesn't require me to do any typing. :) >
Re: [patch mountd.c] replace malloc & memset with calloc
Damn it - that's my bad. I'll stop sending more of these patches (i have a lot) until I get that sorted. Apologies! Quoting Ted Unangst : On Tue, Apr 22, 2014 at 15:49, pe...@petermalone.org wrote: Hi, Another malloc & memset to calloc cleanup. This time in mountd.c Can you be careful about whitespace? Your diffs have spaces instead of tabs and don't apply. The ones you've sent I can hand apply, but for future diffs please work out something that doesn't require me to do any typing. :)
Re: [patch mountd.c] replace malloc & memset with calloc
Thanks Bob. I'll do that. Quoting Bob Beck : Note you can often have this problem if you cut and paste your diff into a mail message Normally best to use something that allows you to just include the diff from a file inline. To check it, mail *yourself* the diff. If you can save the email message raw and apply the diff just by running patch with the email as input, you're good. On Tue, Apr 22, 2014 at 2:23 PM, Ted Unangst wrote: On Tue, Apr 22, 2014 at 15:49, pe...@petermalone.org wrote: Hi, Another malloc & memset to calloc cleanup. This time in mountd.c Can you be careful about whitespace? Your diffs have spaces instead of tabs and don't apply. The ones you've sent I can hand apply, but for future diffs please work out something that doesn't require me to do any typing. :)
Re: reviewing OpenSSL's lib/libssl/src/crypto/asn1
On Mon, Apr 21, 2014 at 05:37, Dirk Engling wrote: > On 21.04.14 04:56, Ted Unangst wrote: > >> Also, can you include diffs inline please? One diff per email. Maybe >> just one or two emails to start, then try sending the rest after we >> see how that goes? > > fix double free in d2i_ASN1_bytes by setting ret->data = NULL after > free, before potential goto err; Applied, and the other one for leaks in err paths. Thanks. Ready for more. :)
Re: reviewing OpenSSL's lib/libssl/src/crypto/asn1
Thanks ted. now I don't have to do it :) Send more diffs Dirk :) On Tue, Apr 22, 2014 at 2:38 PM, Ted Unangst wrote: > On Mon, Apr 21, 2014 at 05:37, Dirk Engling wrote: >> On 21.04.14 04:56, Ted Unangst wrote: >> >>> Also, can you include diffs inline please? One diff per email. Maybe >>> just one or two emails to start, then try sending the rest after we >>> see how that goes? >> >> fix double free in d2i_ASN1_bytes by setting ret->data = NULL after >> free, before potential goto err; > > Applied, and the other one for leaks in err paths. Thanks. > > Ready for more. :) >
Re: Question and regression test for strftime adn wcsftime
On Thu, Apr 17, 2014 at 11:52:09AM -0500, Vladimir Támara Patiño wrote:
> I see a lot of duplication between lib/libc/time/strftime.c and
> lib/libc/time/wcsftime.c.
>
> Since I would like to implement LC_TIME support, I would prefer to
> change only in one place but before that I prepared a simple regression test
> for strftime and wcsftime that is attached.
>
> Checking NetBSD and FreeBSD sources I see they implemented wcsftime by
> translating the wide character strings to multibyte, then calling strftime
> and finally converting the result back to wide character:
> http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libc/locale/wcsftime.c?rev=1.5&content-type=text/x-cvsweb-markup&only_with_tag=MAIN
> https://github.com/freebsd/freebsd/blob/master/lib/libc/locale/wcsftime.c
>
> The drawbacks I see are:
> 1. It is a little less efficient (because it has to translate before
> calling strftime and translate again after)
> 2. If there is no memory or the conversions fail the function must
> return 0 and set errno, and that is not specified in POSIX
> http://pubs.opengroup.org/onlinepubs/009696799/functions/wcsftime.html
>
> What way do you suggest me?
> 1. Implement like FreeBSD and NetBSD did
> 2. Change both functions? they use differente structures lc_time_T that at
> least I would like to unify.
I think a better step forward for LC_TIME support would be to focus on
nl_langinfo() first and make it return locale-specific data for LC_TIME.
The functions you're looking at are icing on the cake. We should be
looking at adding more LC_TIME foundations first.
How and where do you intend to store locale-specific LC_TIME data?
Do you intend to copy FreeBSD's format?
An alternative (but more work) would be to implement a localedef
utility and switch locale definition data to the format specified
by POSIX. I'm not sure how widespread the localedef format really is.
But having a data format that follows the same spec as the functions
using the data might be easier to maintain in the long term.
Your regression test has at least one bug ('bad' is never initialised).
> diff -ruN src56-orig/regress/lib/libc/time/Makefile
> src/regress/lib/libc/time/Makefile
> --- src56-orig/regress/lib/libc/time/Makefile Tue Jan 20 11:47:55 2004
> +++ src/regress/lib/libc/time/MakefileThu Apr 17 08:37:20 2014
> @@ -1,5 +1,5 @@
> #$OpenBSD: Makefile,v 1.1 2004/01/20 16:47:55 millert Exp $
>
> -SUBDIR+=strptime
> +SUBDIR+=ftime strptime
>
> .include
> diff -ruN src56-orig/regress/lib/libc/time/ftime/Makefile
> src/regress/lib/libc/time/ftime/Makefile
> --- src56-orig/regress/lib/libc/time/ftime/Makefile Wed Dec 31 19:00:00 1969
> +++ src/regress/lib/libc/time/ftime/Makefile Thu Apr 17 08:37:05 2014
> @@ -0,0 +1,11 @@
> +
> +NOMAN=
> +PROG=check_ftime
> +
> +CFLAGS=-g
> +
> +
> +run-regress-check_ftime: ${PROG}
> + ./${PROG} >/dev/null
> +
> +.include
> diff -ruN src56-orig/regress/lib/libc/time/ftime/check_ftime.c
> src/regress/lib/libc/time/ftime/check_ftime.c
> --- src56-orig/regress/lib/libc/time/ftime/check_ftime.c Wed Dec 31
> 19:00:00 1969
> +++ src/regress/lib/libc/time/ftime/check_ftime.c Thu Apr 17 11:46:19 2014
> @@ -0,0 +1,268 @@
> +/**
> + * Public domain according to Colombian Legislation.
> + * http://www.pasosdejesus.org/dominio_publico_colombia.html
> + * 2014. vtam...@pasosdejesus.org
> + *
> + * $OpenBSD$
> + */
> +
> +#include
> +#include
> +#include
> +
> +int bad;
> +
> +#define p(t) printf("%s:\t ",#t); \
> + if (t) { \
> + printf("\x1b[38;5;2mOK\x1b[0m\n"); \
> + } else { \
> + bad++; \
> + printf("\x1b[38;5;1mERROR\x1b[0m\n"); \
> + }
> +
> +
> +void test_ftime_posix()
> +{
> + char nom[256];
> + char col[512];
> + wchar_t wcol[512];
> + char *nl;
> + char *enc[]= { "ISO8859-1", "ISO8859-15", "UTF-8" };
> + struct lconv *p;
> + struct tm tl;
> + int i;
> + time_t ti;
> + long ts;
> + for(i = 0; i < sizeof(enc) / sizeof(char *) ; i++) {
> + snprintf(nom, sizeof(nom), "POSIX.%s", enc[i]);
> + printf("nom=%s\n", nom);
> + nl = setlocale(LC_ALL, nom);
> + printf("locale %s\n", nl);
> + p = localeconv();
> + ti = (time_t)139695; //Tue Apr 8 09:40:00 2014
> + gmtime_r(&ti, &tl) ;
> + /*p = strptime("", "", &tm); */
> + strftime(col, sizeof(col), "%A", &tl);
> + p(strcmp(col, "Tuesday") == 0);
> + wcsftime(wcol, sizeof(wcol), L"%A", &tl);
> + p(wcscmp(wcol, L"Tuesday") == 0);
> +
> + strftime(col, sizeof(col), "%a", &tl);
> + p(strcmp(col, "Tue") == 0);
> + wcsftime(wcol, sizeof(wcol), L"%a", &tl);
> + p(wcscmp(wcol, L"Tue") == 0);
> +
> + strftime(col, sizeof(col), "%B", &tl);
> + p(strcmp(col, "April") == 0);
> + wcsftime(wcol, sizeof(wcol), L"%B", &tl);
Re: Question and regression test for strftime adn wcsftime
On Tue, Apr 22, 2014 at 2:43 PM, Stefan Sperling wrote:
> Your regression test has at least one bug ('bad' is never initialised).
It's perhaps bad *style* to not explicitly initialize it, but C99
6.7.8p10 says "If an object that has static storage duration is not
initialized explicitly, then: [...] if it has arithmetic type, it is
initialized to (positive or unsigned) zero". So technically not a
bug. :)
[PATCH]: Add support to Realtek 8168GU to re(4) driver
Hi tech@,
I put my hands today on a Dell Lattitude 3440 and it has an Atheros
AR9565 and a Realtek 8168. Trying -current on it shows up that re(4)
attaches but it cannot recognize the hardware revision and properly
init the card (full dmesg at end):
re0 at pci3 dev 0 function 0 "Realtek 8168" rev 0x10: unknown ASIC
(0x5080), msi, address 00:00:00:00:00:00
rgephy0 at re0 phy 7: RTL8251 PHY, rev. 0
Sprinkling some printf'f in re.c I found that in re_attach there is no
RL_HWREV that matches sc->hw_rev. Actually for this card:
CSR_READ_4(sc, RLTXCFG) = 0x53900d00
RX_TXCFG_HWREV = 0x7c80
sc->hw_rev = 0x5080 (= CSR_READ_4(sc, RLTXCFG) & RX_TXCFG_HWREV)
I searched on FreeBSD re(4) driver and it seems to support this chip
and labels it as 8168GU (see [1] and [2]). The patch below ports these
bits from FreeBSD code but it only uses the flag that OpenBSD re.c
sets for other hardware revisions. After aply the patch the card is
properly recognized and is working, at least it survived two days of
normal use and a full dhclient & pkd_add -u. The same dmesg lines now
becames (real mac address supressed):
re0 at pci3 dev 0 function 0 "Realtek 8168" rev 0x10:
RTL8168GU/8111GU (0x5080), msi, address xx:xx:xx:xx:xx:xx
rgephy0 at re0 phy 7: RTL8251 PHY, rev. 0
Thanks,
Rafael.
References:
[1] http://marc.info/?l=freebsd-commits-all&m=138302463226323&w=2
Patch:
Index: re.c
===
RCS file: /cvs/src/sys/dev/ic/re.c,v
retrieving revision 1.149
diff -u -p -r1.149 re.c
--- re.c13 Mar 2014 13:11:30 - 1.149
+++ re.c21 Apr 2014 04:24:11 -
@@ -240,6 +240,7 @@ static const struct re_revision {
{ RL_HWREV_8168G_SPIN1, "RTL8168G/8111G" },
{ RL_HWREV_8168G_SPIN2, "RTL8168G/8111G" },
{ RL_HWREV_8168G_SPIN4, "RTL8168G/8111G" },
+ { RL_HWREV_8168GU, "RTL8168GU/8111GU" },
{ RL_HWREV_8105E, "RTL8105E" },
{ RL_HWREV_8105E_SPIN1, "RTL8105E" },
{ RL_HWREV_8168D, "RTL8168D/8111D" },
@@ -717,6 +718,7 @@ re_attach(struct rl_softc *sc, const cha
case RL_HWREV_8168G:
case RL_HWREV_8168G_SPIN1:
case RL_HWREV_8168G_SPIN4:
+ case RL_HWREV_8168GU:
sc->rl_flags |= RL_FLAG_INVMAR | RL_FLAG_PHYWAKE |
RL_FLAG_PAR | RL_FLAG_DESCV2 | RL_FLAG_MACSTAT |
RL_FLAG_CMDSTOP | RL_FLAG_AUTOPAD | RL_FLAG_NOJUMBO |
Index: rtl81x9reg.h
===
RCS file: /cvs/src/sys/dev/ic/rtl81x9reg.h,v
retrieving revision 1.80
diff -u -p -r1.80 rtl81x9reg.h
--- rtl81x9reg.h13 Mar 2014 13:11:30 - 1.80
+++ rtl81x9reg.h21 Apr 2014 04:24:11 -
@@ -193,6 +193,7 @@
#define RL_HWREV_8411 0x4880
#define RL_HWREV_8168G 0x4c00
#define RL_HWREV_8168G_SPIN1 0x4c10
+#define RL_HWREV_8168GU0x5080
#define RL_HWREV_8168G_SPIN2 0x5090
#define RL_HWREV_8168G_SPIN4 0x5c80
#define RL_HWREV_8139 0x6000
dmesg and pcidump before patch:
OpenBSD 5.5-current (GENERIC.MP) #74: Sun Apr 20 04:16:25 MDT 2014
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 847760 (8080MB)
avail mem = 8239054848 (7857MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xebc60 (47 entries)
bios0: vendor Dell Inc. version "A05" date 11/15/2013
bios0: Dell Inc. Latitude 3440
acpi0 at bios0: rev 2
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP APIC FPDT ASF! LPIT SSDT SLIC SSDT SSDT MCFG
HPET SSDT SSDT SSDT MSDM SSDT CSRT
acpi0: wakeup devices PXSX(S4) RP01(S3) PXSX(S4) RP02(S3) PXSX(S4)
RP03(S3) PXSX(S4) RP04(S3) RP05(S3) PEGP(S4) PEGA(S4) PXSX(S4)
RP06(S3) PXSX(S4) RP07(S3) PXSX(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM) i7-4500U CPU @ 1.80GHz, 2694.10 MHz
cpu0:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID
cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: Intel(R) Core(TM) i7-4500U CPU @ 1.80GHz, 2693.76 MHz
cpu1:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,MOVBE,POP
Re: Question and regression test for strftime adn wcsftime
On Tue, Apr 22, 2014 at 02:57:17PM -0700, Matthew Dempsky wrote:
> On Tue, Apr 22, 2014 at 2:43 PM, Stefan Sperling wrote:
> > Your regression test has at least one bug ('bad' is never initialised).
>
> It's perhaps bad *style* to not explicitly initialize it, but C99
> 6.7.8p10 says "If an object that has static storage duration is not
> initialized explicitly, then: [...] if it has arithmetic type, it is
> initialized to (positive or unsigned) zero". So technically not a
> bug. :)
Ah. I thought the 'static' keyword was needed for this to apply,
didn't know it affects globals automatically. Thanks for the correction.
[patch complete.c] "never read" variables
Hi tech,
matchlen = 0; is never used.
Fritjof
Index: complete.c
===
RCS file: /cvs/src/usr.bin/ftp/complete.c,v
retrieving revision 1.26
diff -u -p -r1.26 complete.c
--- complete.c 26 Apr 2010 16:51:59 - 1.26
+++ complete.c 22 Apr 2014 23:27:47 -
@@ -90,7 +90,6 @@ complete_ambiguous(char *word, int list,
}
if (!list) {
- matchlen = 0;
lastmatch = words->sl_str[0];
matchlen = strlen(lastmatch);
for (i = 1 ; i < words->sl_cur ; i++) {
[PATCH] remove macros only used once
remove M_ASN1_New_Malloc, M_ASN1_New, M_ASN1_New_Error marcos, they hide
a malloc and are only used once
Index: x_pkey.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/x_pkey.c,v
retrieving revision 1.10
diff -u -r1.10 x_pkey.c
--- x_pkey.c18 Apr 2014 11:20:32 - 1.10
+++ x_pkey.c22 Apr 2014 23:43:03 -
@@ -109,10 +109,18 @@
X509_PKEY *ret = NULL;
ASN1_CTX c;
- M_ASN1_New_Malloc(ret, X509_PKEY);
+ if ((ret = malloc(sizeof(X509_PKEY))) == NULL )
+ ASN1_MAC_H_err(ASN1_F_X509_PKEY_NEW, ERR_R_MALLOC_FAILURE,
+ __LINE__ );
+ return NULL;
+ }
ret->version = 0;
- M_ASN1_New(ret->enc_algor, X509_ALGOR_new);
- M_ASN1_New(ret->enc_pkey, M_ASN1_OCTET_STRING_new);
+ ret->enc_algor = X509_ALGOR_new();
+ if (ret->enc_algor == NULL)
+ return NULL;
+ ret->enc_pkey = M_ASN1_OCTET_STRING_new();
+ if (ret->enc_pkey == NULL)
+ return NULL;
ret->dec_pkey = NULL;
ret->key_length = 0;
ret->key_data = NULL;
@@ -121,7 +129,6 @@
memset(ret->cipher.iv, 0, EVP_MAX_IV_LENGTH);
ret->references = 1;
return (ret);
- M_ASN1_New_Error(ASN1_F_X509_PKEY_NEW);
}
void
Index: asn1_mac.h
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/asn1_mac.h,v
retrieving revision 1.10
diff -u -r1.10 asn1_mac.h
--- asn1_mac.h 17 Apr 2014 13:37:48 - 1.10
+++ asn1_mac.h 22 Apr 2014 23:43:03 -
@@ -287,21 +287,6 @@
c.slen-=(c.p-c.q); \
}
-/* New macros */
-#define M_ASN1_New_Malloc(ret,type) \
- if ((ret=(type *)malloc(sizeof(type))) == NULL) \
- { c.line=__LINE__; goto err2; }
-
-#define M_ASN1_New(arg,func) \
- if (((arg)=func()) == NULL) return(NULL)
-
-#define M_ASN1_New_Error(a) \
-/* err:ASN1_MAC_H_err((a),ERR_R_NESTED_ASN1_ERROR,c.line); \
- return(NULL);*/ \
- err2: ASN1_MAC_H_err((a),ERR_R_MALLOC_FAILURE,c.line); \
- return(NULL)
-
-
/* BIG UGLY WARNING! This is so damn ugly I wanna puke. Unfortunately,
some macros that use ASN1_const_CTX still insist on writing in the input
stream. ARGH! ARGH! ARGH! Let's get rid of this macro package.
Re: [PATCH] remove macros only used once
I will be a minute reading this. The comment from the context at the
bottom of the diff has
me laughing and crying again...
On Tue, Apr 22, 2014 at 5:43 PM, Dirk Engling wrote:
> remove M_ASN1_New_Malloc, M_ASN1_New, M_ASN1_New_Error marcos, they hide
> a malloc and are only used once
>
>
> Index: x_pkey.c
> ===
> RCS file: /cvs/src/lib/libssl/src/crypto/asn1/x_pkey.c,v
> retrieving revision 1.10
> diff -u -r1.10 x_pkey.c
> --- x_pkey.c18 Apr 2014 11:20:32 - 1.10
> +++ x_pkey.c22 Apr 2014 23:43:03 -
> @@ -109,10 +109,18 @@
> X509_PKEY *ret = NULL;
> ASN1_CTX c;
>
> - M_ASN1_New_Malloc(ret, X509_PKEY);
> + if ((ret = malloc(sizeof(X509_PKEY))) == NULL )
> + ASN1_MAC_H_err(ASN1_F_X509_PKEY_NEW, ERR_R_MALLOC_FAILURE,
> + __LINE__ );
> + return NULL;
> + }
> ret->version = 0;
> - M_ASN1_New(ret->enc_algor, X509_ALGOR_new);
> - M_ASN1_New(ret->enc_pkey, M_ASN1_OCTET_STRING_new);
> + ret->enc_algor = X509_ALGOR_new();
> + if (ret->enc_algor == NULL)
> + return NULL;
> + ret->enc_pkey = M_ASN1_OCTET_STRING_new();
> + if (ret->enc_pkey == NULL)
> + return NULL;
> ret->dec_pkey = NULL;
> ret->key_length = 0;
> ret->key_data = NULL;
> @@ -121,7 +129,6 @@
> memset(ret->cipher.iv, 0, EVP_MAX_IV_LENGTH);
> ret->references = 1;
> return (ret);
> - M_ASN1_New_Error(ASN1_F_X509_PKEY_NEW);
> }
>
> void
> Index: asn1_mac.h
> ===
> RCS file: /cvs/src/lib/libssl/src/crypto/asn1/asn1_mac.h,v
> retrieving revision 1.10
> diff -u -r1.10 asn1_mac.h
> --- asn1_mac.h 17 Apr 2014 13:37:48 - 1.10
> +++ asn1_mac.h 22 Apr 2014 23:43:03 -
> @@ -287,21 +287,6 @@
> c.slen-=(c.p-c.q); \
> }
>
> -/* New macros */
> -#define M_ASN1_New_Malloc(ret,type) \
> - if ((ret=(type *)malloc(sizeof(type))) == NULL) \
> - { c.line=__LINE__; goto err2; }
> -
> -#define M_ASN1_New(arg,func) \
> - if (((arg)=func()) == NULL) return(NULL)
> -
> -#define M_ASN1_New_Error(a) \
> -/* err:ASN1_MAC_H_err((a),ERR_R_NESTED_ASN1_ERROR,c.line); \
> - return(NULL);*/ \
> - err2: ASN1_MAC_H_err((a),ERR_R_MALLOC_FAILURE,c.line); \
> - return(NULL)
> -
> -
> /* BIG UGLY WARNING! This is so damn ugly I wanna puke. Unfortunately,
> some macros that use ASN1_const_CTX still insist on writing in the input
> stream. ARGH! ARGH! ARGH! Let's get rid of this macro package.
>
Re: [PATCH] usr.bin/sdiff/sdiff.c prompt bikeshedding
Ok, this time with more mutt:
Index: usr.bin/sdiff//sdiff.c
===
RCS file: /work/cvsroot/src/usr.bin/sdiff/sdiff.c,v
retrieving revision 1.30
diff -p -u -r1.30 sdiff.c
--- usr.bin/sdiff//sdiff.c 26 Nov 2013 21:08:12 - 1.30
+++ usr.bin/sdiff//sdiff.c 22 Apr 2014 18:27:38 -
@@ -421,6 +421,7 @@ prompt(const char *s1, const char *s2)
/* Print command prompt. */
putchar('%');
+ putchar(' ');
/* Get user input. */
for (; (cmd = xfgets(stdin)); free(cmd)) {
@@ -474,6 +475,7 @@ USAGE:
int_usage();
PROMPT:
putchar('%');
+ putchar(' ');
/* Prompt user again. */
continue;
On Tue, Apr 22, 2014 at 01:33:48PM -0500, Kent R. Spillner wrote:
> Sorry, my webmail client ate the whitespace. mutt is a casualty of the
> Valhalla rampage (whines about permissions on /dev/arandom) and kerberosV
> removal. I'll rebuild mutt from ports and resubmit.
>
>
> -Original Message-
> From: "Kent R. Spillner"
> Sent: Tuesday, April 22, 2014 13:29
> To: tech@openbsd.org
> Subject: [PATCH] usr.bin/sdiff/sdiff.c prompt bikeshedding
>
> Personally, when sysmerge asks if I'd like to merge something I'd
> prefer a little breathing room at the prompt. (Will this break
> anything?)
>
>
> Index: usr.bin/sdiff//sdiff.c
> ===
> RCS file: /work/cvsroot/src/usr.bin/sdiff/sdiff.c,v
> retrieving revision 1.30
> diff -p -u -r1.30 sdiff.c
> --- usr.bin/sdiff//sdiff.c 26 Nov 2013 21:08:12 - 1.30
> +++ usr.bin/sdiff//sdiff.c 22 Apr 2014 18:27:38 -
> @@ -421,6 +421,7 @@ prompt(const char *s1, const char *s2)
>
> /* Print command prompt. */
> putchar('%');
> + putchar(' ');
>
> /* Get user input. */
> for (; (cmd = xfgets(stdin)); free(cmd)) {
> @@ -474,6 +475,7 @@ USAGE:
> int_usage();
> PROMPT:
> putchar('%');
> + putchar(' ');
>
> /* Prompt user again. */
> continue;
>
>
>
>
>
>
bpf(4) obsolete data-link levels
If I'm not mistaken, we had no drivers left that use those types? (I've only found references to the now defunct pdq and midway.) Index: bpf.c === RCS file: /cvs/src/sys/net/bpf.c,v retrieving revision 1.92 diff -u -p -p -u -r1.92 bpf.c --- bpf.c 14 Apr 2014 09:06:42 - 1.92 +++ bpf.c 22 Apr 2014 23:29:41 - @@ -152,12 +152,6 @@ bpf_movein(struct uio *uio, u_int linkty hlen = ETHER_HDR_LEN; break; - case DLT_FDDI: - sockp->sa_family = AF_UNSPEC; - /* XXX 4(FORMAC)+6(dst)+6(src)+3(LLC)+5(SNAP) */ - hlen = 24; - break; - case DLT_IEEE802_11: case DLT_IEEE802_11_RADIO: sockp->sa_family = AF_UNSPEC; @@ -168,16 +162,6 @@ bpf_movein(struct uio *uio, u_int linkty case DLT_NULL: sockp->sa_family = AF_UNSPEC; hlen = 0; - break; - - case DLT_ATM_RFC1483: - /* -* An ATM driver requires 4-byte ATM pseudo header. -* Though it isn't standard, vpi:vci needs to be -* specified anyway. -*/ - sockp->sa_family = AF_UNSPEC; - hlen = 12; /* XXX 4(ATM_PH) + 3(LLC) + 5(SNAP) */ break; default: -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: [PATCH] remove macros only used once
Bob Beck writes:
> I will be a minute reading this. The comment from the context at the
> bottom of the diff has
> me laughing and crying again...
Note that asn1_mac.h is installed...
> On Tue, Apr 22, 2014 at 5:43 PM, Dirk Engling wrote:
>> remove M_ASN1_New_Malloc, M_ASN1_New, M_ASN1_New_Error marcos, they hide
>> a malloc and are only used once
>>
>>
>> Index: x_pkey.c
>> ===
>> RCS file: /cvs/src/lib/libssl/src/crypto/asn1/x_pkey.c,v
>> retrieving revision 1.10
>> diff -u -r1.10 x_pkey.c
>> --- x_pkey.c18 Apr 2014 11:20:32 - 1.10
>> +++ x_pkey.c22 Apr 2014 23:43:03 -
>> @@ -109,10 +109,18 @@
>> X509_PKEY *ret = NULL;
>> ASN1_CTX c;
>>
>> - M_ASN1_New_Malloc(ret, X509_PKEY);
>> + if ((ret = malloc(sizeof(X509_PKEY))) == NULL )
>> + ASN1_MAC_H_err(ASN1_F_X509_PKEY_NEW, ERR_R_MALLOC_FAILURE,
>> + __LINE__ );
>> + return NULL;
>> + }
>> ret->version = 0;
>> - M_ASN1_New(ret->enc_algor, X509_ALGOR_new);
>> - M_ASN1_New(ret->enc_pkey, M_ASN1_OCTET_STRING_new);
>> + ret->enc_algor = X509_ALGOR_new();
>> + if (ret->enc_algor == NULL)
>> + return NULL;
>> + ret->enc_pkey = M_ASN1_OCTET_STRING_new();
>> + if (ret->enc_pkey == NULL)
>> + return NULL;
>> ret->dec_pkey = NULL;
>> ret->key_length = 0;
>> ret->key_data = NULL;
>> @@ -121,7 +129,6 @@
>> memset(ret->cipher.iv, 0, EVP_MAX_IV_LENGTH);
>> ret->references = 1;
>> return (ret);
>> - M_ASN1_New_Error(ASN1_F_X509_PKEY_NEW);
>> }
>>
>> void
>> Index: asn1_mac.h
>> ===
>> RCS file: /cvs/src/lib/libssl/src/crypto/asn1/asn1_mac.h,v
>> retrieving revision 1.10
>> diff -u -r1.10 asn1_mac.h
>> --- asn1_mac.h 17 Apr 2014 13:37:48 - 1.10
>> +++ asn1_mac.h 22 Apr 2014 23:43:03 -
>> @@ -287,21 +287,6 @@
>> c.slen-=(c.p-c.q); \
>> }
>>
>> -/* New macros */
>> -#define M_ASN1_New_Malloc(ret,type) \
>> - if ((ret=(type *)malloc(sizeof(type))) == NULL) \
>> - { c.line=__LINE__; goto err2; }
>> -
>> -#define M_ASN1_New(arg,func) \
>> - if (((arg)=func()) == NULL) return(NULL)
>> -
>> -#define M_ASN1_New_Error(a) \
>> -/* err:ASN1_MAC_H_err((a),ERR_R_NESTED_ASN1_ERROR,c.line); \
>> - return(NULL);*/ \
>> - err2: ASN1_MAC_H_err((a),ERR_R_MALLOC_FAILURE,c.line); \
>> - return(NULL)
>> -
>> -
>> /* BIG UGLY WARNING! This is so damn ugly I wanna puke. Unfortunately,
>> some macros that use ASN1_const_CTX still insist on writing in the input
>> stream. ARGH! ARGH! ARGH! Let's get rid of this macro package.
>>
>
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
patch for libressl apps/speed.c
I believe the %u format is incorrect in these cases as these are signed ints. Brad Index: apps/speed.c === RCS file: /cvs/src/lib/libssl/src/apps/speed.c,v retrieving revision 1.34 diff -u -r1.34 speed.c --- apps/speed.c 22 Apr 2014 14:54:13 - 1.34 +++ apps/speed.c 23 Apr 2014 00:06:36 - @@ -2250,7 +2250,7 @@ j = 0; } if (mr) - fprintf(stdout, "+F2:%u:%u:%f:%f\n", + fprintf(stdout, "+F2:%i:%u:%f:%f\n", k, rsa_bits[k], rsa_results[k][0], rsa_results[k][1]); else @@ -2269,7 +2269,7 @@ j = 0; } if (mr) - fprintf(stdout, "+F3:%u:%u:%f:%f\n", + fprintf(stdout, "+F3:%i:%u:%f:%f\n", k, dsa_bits[k], dsa_results[k][0], dsa_results[k][1]); else fprintf(stdout, "dsa %4u bits %8.6fs %8.6fs %8.1f %8.1f\n", @@ -2287,12 +2287,12 @@ j = 0; } if (mr) - fprintf(stdout, "+F4:%u:%u:%f:%f\n", + fprintf(stdout, "+F4:%i:%i:%f:%f\n", k, test_curves_bits[k], ecdsa_results[k][0], ecdsa_results[k][1]); else fprintf(stdout, - "%4u bit ecdsa (%s) %8.4fs %8.4fs %8.1f %8.1f\n", + "%4i bit ecdsa (%s) %8.4fs %8.4fs %8.1f %8.1f\n", test_curves_bits[k], test_curves_names[k], ecdsa_results[k][0], ecdsa_results[k][1], @@ -2311,12 +2311,12 @@ j = 0; } if (mr) - fprintf(stdout, "+F5:%u:%u:%f:%f\n", + fprintf(stdout, "+F5:%i:%i:%f:%f\n", k, test_curves_bits[k], ecdh_results[k][0], 1.0 / ecdh_results[k][0]); else - fprintf(stdout, "%4u bit ecdh (%s) %8.4fs %8.1f\n", + fprintf(stdout, "%4i bit ecdh (%s) %8.4fs %8.1f\n", test_curves_bits[k], test_curves_names[k], ecdh_results[k][0], 1.0 / ecdh_results[k][0]);
Re: [PATCH] remove macros only used once
Note that asn1_mac.h is installed... You're right, I found it referenced at least here: http://opensource.apple.com/source/OpenSSL/OpenSSL-5/openssl/crypto/asn1/p5_pbev2.c erdgeist
Re: [PATCH] remove macros only used once
I hate the amount of useless garbage API this thing exposes externally, that we then have to wonder "WTF out there might use it..".. G. Dirk the right way to do this is leave the macros for now (ick) but change the internals of all our stuff to use intrinsics without the use of the macros. We don't want to be monkeying with the visible API, much, if we can help it. On Tue, Apr 22, 2014 at 6:16 PM, Dirk Engling wrote: > >> Note that asn1_mac.h is installed... > > > You're right, I found it referenced at least here: > > http://opensource.apple.com/source/OpenSSL/OpenSSL-5/openssl/crypto/asn1/p5_pbev2.c > > erdgeist >
Re: [PATCH] remove macros only used once
Once we have that done we might at an opportune time ask our intrepid ports builders to find out for us who is using that little gem, and we could *conisder* removing it then if we're certain nobody has walked in looking like an Oatmeal-drawn dog and said "Oh boy Oh boy.. ANOTHER malloc wrapper I can use! I must use it!" On Tue, Apr 22, 2014 at 6:23 PM, Bob Beck wrote: > I hate the amount of useless garbage API this thing exposes > externally, that we then have to wonder "WTF out there might use > it..".. G. > > Dirk the right way to do this is leave the macros for now (ick) but > change the internals of all our stuff to use intrinsics > without the use of the macros. We don't want to be monkeying with the > visible API, much, if we can help it. > > > On Tue, Apr 22, 2014 at 6:16 PM, Dirk Engling wrote: >> >>> Note that asn1_mac.h is installed... >> >> >> You're right, I found it referenced at least here: >> >> http://opensource.apple.com/source/OpenSSL/OpenSSL-5/openssl/crypto/asn1/p5_pbev2.c >> >> erdgeist >>
[PATCH] cleanup libssl/src/crypto/asn1/a_{int,enum}.c
* remove unnecessary temp variable d
* move loop counter j in for() header
* fix prototype for memcpy
* make calculation of actual length in BN_to_ASN1_ENUMERATED
more transparent
This code still looks rather odd, it uses a temporary buffer to first
convert the number into a minimal little endian representation and then
reverts the bytes to minimal big endian representation. A simple clz()
could have revealed the number of bytes necessary to encode. I will fix
this later.
Index: a_enum.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_enum.c,v
retrieving revision 1.12
diff -u -r1.12 a_enum.c
--- a_enum.c21 Apr 2014 11:37:41 - 1.12
+++ a_enum.c23 Apr 2014 01:30:43 -
@@ -72,7 +72,6 @@
int j, k;
unsigned int i;
unsigned char buf[sizeof(long) + 1];
- long d;
a->type = V_ASN1_ENUMERATED;
if (a->length < (int)(sizeof(long) + 1)) {
@@ -84,20 +83,18 @@
ASN1err(ASN1_F_ASN1_ENUMERATED_SET, ERR_R_MALLOC_FAILURE);
return (0);
}
- d = v;
- if (d < 0) {
- d = -d;
+ if (v < 0) {
+ v = -v;
a->type = V_ASN1_NEG_ENUMERATED;
}
for (i = 0; i < sizeof(long); i++) {
- if (d == 0)
+ if (v == 0)
break;
- buf[i] = (int)d & 0xff;
- d >>= 8;
+ buf[i] = (unsigned char)v;
+ v >>= 8;
}
- j = 0;
- for (k = i - 1; k >=0; k--)
+ for (j = 0, k = i - 1; k >=0; k--)
a->data[j++] = buf[k];
a->length = j;
return (1);
@@ -137,7 +134,7 @@
BN_to_ASN1_ENUMERATED(BIGNUM *bn, ASN1_ENUMERATED *ai)
{
ASN1_ENUMERATED *ret;
- int len, j;
+ int len;
if (ai == NULL)
ret = M_ASN1_ENUMERATED_new();
@@ -151,10 +148,10 @@
ret->type = V_ASN1_NEG_ENUMERATED;
else
ret->type = V_ASN1_ENUMERATED;
- j = BN_num_bits(bn);
- len = ((j == 0) ? 0 : ((j / 8) + 1));
- if (ret->length < len + 4) {
- unsigned char *new_data = realloc(ret->data, len + 4);
+ len = BN_num_bits(bn);
+ len = 4 + ( len ? len / 8 + 1 : 0 );
+ if (ret->length < len) {
+ unsigned char *new_data = realloc(ret->data, len);
if (!new_data) {
ASN1err(ASN1_F_BN_TO_ASN1_ENUMERATED,
ERR_R_MALLOC_FAILURE);
goto err;
Index: a_int.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_int.c,v
retrieving revision 1.18
diff -u -r1.18 a_int.c
--- a_int.c 21 Apr 2014 11:37:41 - 1.18
+++ a_int.c 23 Apr 2014 01:30:43 -
@@ -159,7 +159,7 @@
if (a->length == 0)
*(p++) = 0;
else if (!neg)
- memcpy(p, a->data, (unsigned int)a->length);
+ memcpy(p, a->data, (size_t)a->length);
else {
/* Begin at the end of the encoding */
n = a->data + a->length - 1;
@@ -346,7 +346,6 @@
int j, k;
unsigned int i;
unsigned char buf[sizeof(long) + 1];
- long d;
a->type = V_ASN1_INTEGER;
if (a->length < (int)(sizeof(long) + 1)) {
@@ -358,20 +357,18 @@
ASN1err(ASN1_F_ASN1_INTEGER_SET, ERR_R_MALLOC_FAILURE);
return (0);
}
- d = v;
- if (d < 0) {
- d = -d;
+ if (v < 0) {
+ v = -v;
a->type = V_ASN1_NEG_INTEGER;
}
for (i = 0; i < sizeof(long); i++) {
- if (d == 0)
+ if (v == 0)
break;
- buf[i] = (int)d & 0xff;
- d >>= 8;
+ buf[i] = (int)v & 0xff;
+ v >>= 8;
}
- j = 0;
- for (k = i - 1; k >= 0; k--)
+ for (j = 0, k = i - 1; k >= 0; k--)
a->data[j++] = buf[k];
a->length = j;
return (1);
@@ -411,7 +408,7 @@
BN_to_ASN1_INTEGER(const BIGNUM *bn, ASN1_INTEGER *ai)
{
ASN1_INTEGER *ret;
- int len, j;
+ int len;
if (ai == NULL)
ret = M_ASN1_INTEGER_new();
@@ -425,10 +422,10 @@
ret->type = V_ASN1_NEG_INTEGER;
else
ret->type = V_ASN1_INTEGER;
- j = BN_num_bits(bn);
- len = ((j == 0) ? 0 : ((j / 8) + 1));
- if (ret->length < len + 4) {
- unsigned char *new_data = realloc(ret->data, len + 4);
+ len = BN_num_bits(bn);
+ len = 4 + ( len ? len / 8 + 1 : 0 );
+ if (ret->length < len) {
+ unsigned char *new_data = realloc(ret->data, len);
if (!new_data) {
ASN1err(ASN1_F_BN_TO_ASN1_INTEGER,
ERR_R_MALLOC_FAILURE);
goto e
[PATCH] remove unnecessary second NULL assignment
Index: tasn_fre.c === RCS file: /cvs/src/lib/libssl/src/crypto/asn1/tasn_fre.c,v retrieving revision 1.9 diff -u -r1.9 tasn_fre.c --- tasn_fre.c 18 Apr 2014 12:15:48 - 1.9 +++ tasn_fre.c 23 Apr 2014 01:47:49 - @@ -247,7 +247,6 @@ default: ASN1_STRING_free((ASN1_STRING *)*pval); - *pval = NULL; break; } *pval = NULL;
Re: mo junk mo problems
On Mon, Apr 14, 2014 at 12:12, Otto Moerbeek wrote:
> On Sun, Apr 13, 2014 at 06:34:17PM -0400, Ted Unangst wrote:
>
>> I took another look at the way junk works in malloc, and there's a few
>> improvements I'd like to make.
>> I have't been able to build perl reliably with this diff, but haven't
>> ruled out the 5.18 update either.
This was just update/make -j 2 wonkiness. All good now.
> - always junk half a page? Maybe that's a tad over done, athough it
> might be relatively cheap since your writing the page anyway.
>
> - Chunks are now always completely junked on free, maybe a bit
> overdone for malloc_junk = 1
It doesn't seem any slower. I'd like to commit this, then see if
anybody complains loud enough for me to hear them. :) hard to get
consistent build times these days...
> - if we are freeing a large region, it does not end up in the cache,
> so junking (part of) it is a waste of effort. (That already is the
> case for current, you are not introducing that).
There's a few other things I'd like to change about the freelist. Next
diff.
> - What about 'j'? Do we want it to cancel a J or always disable
> junking?
I think disable entirely is the right answer.
Here's the diff again. I was going to update it, but I don't see
anything I want to change. I think it's fine as is for the first round.
(The man page parts already snuck in.)
Index: malloc.c
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.152
diff -u -p -r1.152 malloc.c
--- malloc.c3 Apr 2014 16:18:11 - 1.152
+++ malloc.c13 Apr 2014 21:05:11 -
@@ -167,7 +167,6 @@ struct malloc_readonly {
int malloc_move;/* move allocations to end of page? */
int malloc_realloc; /* always realloc? */
int malloc_xmalloc; /* xmalloc behaviour? */
- int malloc_zero;/* zero fill? */
size_t malloc_guard; /* use guard pages after allocations? */
u_int malloc_cache; /* free pages we cache */
#ifdef MALLOC_STATS
@@ -412,7 +411,7 @@ map(struct dir_info *d, size_t sz, int z
d->free_regions_size -= psz;
if (zero_fill)
memset(p, 0, sz);
- else if (mopts.malloc_junk &&
+ else if (mopts.malloc_junk == 2 &&
mopts.malloc_freeunmap)
memset(p, SOME_FREEJUNK, sz);
return p;
@@ -431,7 +430,7 @@ map(struct dir_info *d, size_t sz, int z
d->free_regions_size -= psz;
if (zero_fill)
memset(p, 0, sz);
- else if (mopts.malloc_junk && mopts.malloc_freeunmap)
+ else if (mopts.malloc_junk == 2 && mopts.malloc_freeunmap)
memset(p, SOME_FREEJUNK, sz);
return p;
}
@@ -461,6 +460,7 @@ omalloc_init(struct dir_info **dp)
* Default options
*/
mopts.malloc_abort = 1;
+ mopts.malloc_junk = 1;
mopts.malloc_move = 1;
mopts.malloc_cache = MALLOC_DEFAULT_CACHE;
@@ -534,7 +534,7 @@ omalloc_init(struct dir_info **dp)
mopts.malloc_junk = 0;
break;
case 'J':
- mopts.malloc_junk = 1;
+ mopts.malloc_junk = 2;
break;
case 'n':
case 'N':
@@ -557,7 +557,8 @@ omalloc_init(struct dir_info **dp)
mopts.malloc_cache = MALLOC_DEFAULT_CACHE;
break;
case 'S':
- mopts.malloc_freeunmap = mopts.malloc_junk = 1;
+ mopts.malloc_freeunmap = 1;
+ mopts.malloc_junk = 2;
mopts.malloc_guard = MALLOC_PAGESIZE;
mopts.malloc_cache = 0;
break;
@@ -573,12 +574,6 @@ omalloc_init(struct dir_info **dp)
case 'X':
mopts.malloc_xmalloc = 1;
break;
- case 'z':
- mopts.malloc_zero = 0;
- break;
- case 'Z':
- mopts.malloc_zero = 1;
- break;
default: {
static const char q[] = "malloc() warning: "
"unknown char in MALLOC_OPTIONS\n";
@@ -589,13 +584,6 @@ omalloc_init(struct dir_info **dp)
}
}
- /*
[PATCH] Use sizeof() to pass buffer size to cleanser
Index: n_pkey.c === RCS file: /cvs/src/lib/libssl/src/crypto/asn1/n_pkey.c,v retrieving revision 1.15 diff -u -r1.15 n_pkey.c --- n_pkey.c21 Apr 2014 11:37:41 - 1.15 +++ n_pkey.c23 Apr 2014 01:50:02 - @@ -205,7 +205,7 @@ if (!EVP_BytesToKey(EVP_rc4(), EVP_md5(), NULL, buf, i,1, key, NULL)) goto err; - OPENSSL_cleanse(buf, 256); + OPENSSL_cleanse(buf, sizeof(buf)); /* Encrypt private key in place */ zz = enckey->enckey->digest->data; @@ -302,7 +302,7 @@ if (!EVP_BytesToKey(EVP_rc4(), EVP_md5(), NULL, buf, i,1, key, NULL)) goto err; - OPENSSL_cleanse(buf, 256); + OPENSSL_cleanse(buf, sizeof(buf)); if (!EVP_DecryptInit_ex(&ctx, EVP_rc4(), NULL, key, NULL)) goto err;
[PATCH| zero a freed pointer passed in a struct, to prevent reuse after free
Index: x_x509.c === RCS file: /cvs/src/lib/libssl/src/crypto/asn1/x_x509.c,v retrieving revision 1.12 diff -u -r1.12 x_x509.c --- x_x509.c18 Apr 2014 11:20:32 - 1.12 +++ x_x509.c23 Apr 2014 01:54:03 - @@ -125,6 +125,7 @@ #endif if (ret->name != NULL) free(ret->name); + ret->name = NULL; break; }
small patch: CRYPTO_memcmp
Hi tech@,
Sending this patch for comment...
CRYPTO_memcmp() is different to memcmp() because it can only check
for equality, not greater-than/less-than.
If we check the string in reverse order we can remove a variable
from the comparison loop.
Does this look ok?
- Michael
Index: cryptlib.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/cryptlib.c,v
retrieving revision 1.23
diff -u -r1.23 cryptlib.c
--- cryptlib.c 21 Apr 2014 11:19:28 - 1.23
+++ cryptlib.c 23 Apr 2014 01:19:39 -
@@ -727,15 +727,13 @@
}
int
-CRYPTO_memcmp(const void *in_a, const void *in_b, size_t len)
+CRYPTO_memcmp(const void *in_a, const void *in_b, size_t n)
{
- size_t i;
const unsigned char *a = in_a;
const unsigned char *b = in_b;
unsigned char x = 0;
- for (i = 0; i < len; i++)
- x |= a[i] ^ b[i];
-
+ while (n-- > 0)
+ x |= a[n] ^ b[n];
return x;
}
Re: [PATCH]: Add support to Realtek 8168GU to re(4) driver
Thanks committed. This would have been matched by RL_HWREV_8168G_SPIN2 had I not forgotten to mask the define with 0x7c80 when adding it. On Tue, Apr 22, 2014 at 06:02:41PM -0400, Rafael Neves wrote: > Hi tech@, > > I put my hands today on a Dell Lattitude 3440 and it has an Atheros > AR9565 and a Realtek 8168. Trying -current on it shows up that re(4) > attaches but it cannot recognize the hardware revision and properly > init the card (full dmesg at end): > re0 at pci3 dev 0 function 0 "Realtek 8168" rev 0x10: unknown ASIC > (0x5080), msi, address 00:00:00:00:00:00 > rgephy0 at re0 phy 7: RTL8251 PHY, rev. 0 > > Sprinkling some printf'f in re.c I found that in re_attach there is no > RL_HWREV that matches sc->hw_rev. Actually for this card: > CSR_READ_4(sc, RLTXCFG) = 0x53900d00 > RX_TXCFG_HWREV = 0x7c80 > sc->hw_rev = 0x5080 (= CSR_READ_4(sc, RLTXCFG) & RX_TXCFG_HWREV) > > I searched on FreeBSD re(4) driver and it seems to support this chip > and labels it as 8168GU (see [1] and [2]). The patch below ports these > bits from FreeBSD code but it only uses the flag that OpenBSD re.c > sets for other hardware revisions. After aply the patch the card is > properly recognized and is working, at least it survived two days of > normal use and a full dhclient & pkd_add -u. The same dmesg lines now > becames (real mac address supressed): > re0 at pci3 dev 0 function 0 "Realtek 8168" rev 0x10: > RTL8168GU/8111GU (0x5080), msi, address xx:xx:xx:xx:xx:xx > rgephy0 at re0 phy 7: RTL8251 PHY, rev. 0 > > Thanks, > Rafael. > > References: >[1] http://marc.info/?l=freebsd-commits-all&m=138302463226323&w=2
Re: small patch: CRYPTO_memcmp
Nope. One of those things is not like the other..
On Tue, Apr 22, 2014 at 7:05 PM, Michael W. Bombardieri wrote:
> Hi tech@,
>
> Sending this patch for comment...
>
> CRYPTO_memcmp() is different to memcmp() because it can only check
> for equality, not greater-than/less-than.
> If we check the string in reverse order we can remove a variable
> from the comparison loop.
>
> Does this look ok?
>
> - Michael
>
>
>
> Index: cryptlib.c
> ===
> RCS file: /cvs/src/lib/libssl/src/crypto/cryptlib.c,v
> retrieving revision 1.23
> diff -u -r1.23 cryptlib.c
> --- cryptlib.c 21 Apr 2014 11:19:28 - 1.23
> +++ cryptlib.c 23 Apr 2014 01:19:39 -
> @@ -727,15 +727,13 @@
> }
>
> int
> -CRYPTO_memcmp(const void *in_a, const void *in_b, size_t len)
> +CRYPTO_memcmp(const void *in_a, const void *in_b, size_t n)
> {
> - size_t i;
> const unsigned char *a = in_a;
> const unsigned char *b = in_b;
> unsigned char x = 0;
>
> - for (i = 0; i < len; i++)
> - x |= a[i] ^ b[i];
> -
> + while (n-- > 0)
> + x |= a[n] ^ b[n];
> return x;
> }
>
Re: [PATCH] Use sizeof() to pass buffer size to cleanser
Looks good, but if you chase something like this, it's ok to send a
diff that kills all of them at once in the same file, like this. which
replaces everywhere the original author didn't know about sizeof(buf)
:)
-Bob
Index: n_pkey.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/n_pkey.c,v
retrieving revision 1.15
diff -u -p -u -p -r1.15 n_pkey.c
--- n_pkey.c21 Apr 2014 11:37:41 - 1.15
+++ n_pkey.c23 Apr 2014 04:14:39 -
@@ -189,7 +189,7 @@ i2d_RSA_NET(const RSA *a, unsigned char
if (cb == NULL)
cb = EVP_read_pw_string;
- i = cb((char *)buf, 256, "Enter Private Key password:", 1);
+ i = cb((char *)buf, sizeof(buf), "Enter Private Key password:", 1);
if (i != 0) {
ASN1err(ASN1_F_I2D_RSA_NET, ASN1_R_BAD_PASSWORD_READ);
goto err;
@@ -205,7 +205,7 @@ i2d_RSA_NET(const RSA *a, unsigned char
if (!EVP_BytesToKey(EVP_rc4(), EVP_md5(), NULL, buf, i,1, key, NULL))
goto err;
- OPENSSL_cleanse(buf, 256);
+ OPENSSL_cleanse(buf, sizeof(buf));
/* Encrypt private key in place */
zz = enckey->enckey->digest->data;
@@ -286,7 +286,7 @@ d2i_RSA_NET_2(RSA **a, ASN1_OCTET_STRING
EVP_CIPHER_CTX ctx;
EVP_CIPHER_CTX_init(&ctx);
- i=cb((char *)buf,256, "Enter Private Key password:",0);
+ i=cb((char *)buf, sizeof(buf), "Enter Private Key password:",0);
if (i != 0) {
ASN1err(ASN1_F_D2I_RSA_NET_2, ASN1_R_BAD_PASSWORD_READ);
goto err;
@@ -302,7 +302,7 @@ d2i_RSA_NET_2(RSA **a, ASN1_OCTET_STRING
if (!EVP_BytesToKey(EVP_rc4(), EVP_md5(), NULL, buf, i,1, key, NULL))
goto err;
- OPENSSL_cleanse(buf, 256);
+ OPENSSL_cleanse(buf, sizeof(buf));
if (!EVP_DecryptInit_ex(&ctx, EVP_rc4(), NULL, key, NULL))
goto err;
Re: small patch: CRYPTO_memcmp
On Wed, Apr 23, 2014 at 09:05, Michael W. Bombardieri wrote: > CRYPTO_memcmp() is different to memcmp() because it can only check > for equality, not greater-than/less-than. > If we check the string in reverse order we can remove a variable > from the comparison loop. > > Does this look ok? Almost, but... > + while (n-- > 0) > + x |= a[n] ^ b[n]; Won't compare the bytes at [0]. I think switching this to be timingsafe_bcmp would be better, then we only have copy.
Re: [PATCH| zero a freed pointer passed in a struct, to prevent reuse after free
Yes, ok committed On Wed, Apr 23, 2014 at 03:55:19AM +0200, Dirk Engling wrote: > Index: x_x509.c > === > RCS file: /cvs/src/lib/libssl/src/crypto/asn1/x_x509.c,v > retrieving revision 1.12 > diff -u -r1.12 x_x509.c > --- x_x509.c 18 Apr 2014 11:20:32 - 1.12 > +++ x_x509.c 23 Apr 2014 01:54:03 - > @@ -125,6 +125,7 @@ > #endif > if (ret->name != NULL) > free(ret->name); > + ret->name = NULL; > break; > } >
Re: small patch: CRYPTO_memcmp
> > + while (n-- > 0) > > + x |= a[n] ^ b[n]; > > Won't compare the bytes at [0]. Uh? It will, n gets decremented after the test but before the x |= statement. > I think switching this to be > timingsafe_bcmp would be better, then we only have copy. Agreed.
Re: [PATCH]: Add support to Realtek 8168GU to re(4) driver
Hi, You are welcome. And thank you for looking at this! On Tue, Apr 22, 2014 at 11:45 PM, Jonathan Gray wrote: > Thanks committed. This would have been matched by > RL_HWREV_8168G_SPIN2 had I not forgotten to mask the define > with 0x7c80 when adding it. > > On Tue, Apr 22, 2014 at 06:02:41PM -0400, Rafael Neves wrote: >> Hi tech@, >> >> I put my hands today on a Dell Lattitude 3440 and it has an Atheros >> AR9565 and a Realtek 8168. Trying -current on it shows up that re(4) >> attaches but it cannot recognize the hardware revision and properly >> init the card (full dmesg at end): >> re0 at pci3 dev 0 function 0 "Realtek 8168" rev 0x10: unknown ASIC >> (0x5080), msi, address 00:00:00:00:00:00 >> rgephy0 at re0 phy 7: RTL8251 PHY, rev. 0 >> >> Sprinkling some printf'f in re.c I found that in re_attach there is no >> RL_HWREV that matches sc->hw_rev. Actually for this card: >> CSR_READ_4(sc, RLTXCFG) = 0x53900d00 >> RX_TXCFG_HWREV = 0x7c80 >> sc->hw_rev = 0x5080 (= CSR_READ_4(sc, RLTXCFG) & RX_TXCFG_HWREV) >> >> I searched on FreeBSD re(4) driver and it seems to support this chip >> and labels it as 8168GU (see [1] and [2]). The patch below ports these >> bits from FreeBSD code but it only uses the flag that OpenBSD re.c >> sets for other hardware revisions. After aply the patch the card is >> properly recognized and is working, at least it survived two days of >> normal use and a full dhclient & pkd_add -u. The same dmesg lines now >> becames (real mac address supressed): >> re0 at pci3 dev 0 function 0 "Realtek 8168" rev 0x10: >> RTL8168GU/8111GU (0x5080), msi, address xx:xx:xx:xx:xx:xx >> rgephy0 at re0 phy 7: RTL8251 PHY, rev. 0 >> >> Thanks, >> Rafael. >> >> References: >>[1] http://marc.info/?l=freebsd-commits-all&m=138302463226323&w=2
Re: small patch: CRYPTO_memcmp
On Wed, Apr 23, 2014 at 04:39:01AM +, Miod Vallat wrote: > > > + while (n-- > 0) > > > + x |= a[n] ^ b[n]; > > > > Won't compare the bytes at [0]. > > Uh? It will, n gets decremented after the test but before the x |= > statement. Heh. you're right. And both Ted and I were dumbasses. I have tied the Mr. Gumby hankerchief to my head. Although probably means that hack is really not readable enough to be worth saving 8 bytes of stack ;) > > > I think switching this to be > > timingsafe_bcmp would be better, then we only have copy. > > Agreed. > Yup.
Re: [PATCH| zero a freed pointer passed in a struct, to prevent reuse after free
On Tue, Apr 22, 2014 at 10:33:55PM -0600, Bob Beck wrote: > Yes, ok > > committed > > > On Wed, Apr 23, 2014 at 03:55:19AM +0200, Dirk Engling wrote: > > Index: x_x509.c > > === > > RCS file: /cvs/src/lib/libssl/src/crypto/asn1/x_x509.c,v > > retrieving revision 1.12 > > diff -u -r1.12 x_x509.c > > --- x_x509.c18 Apr 2014 11:20:32 - 1.12 > > +++ x_x509.c23 Apr 2014 01:54:03 - > > @@ -125,6 +125,7 @@ > > #endif > > if (ret->name != NULL) > > free(ret->name); > > + ret->name = NULL; > > break; > > } Why not kill the 'if (ret->name != NULL)' check while at it?
Re: mo junk mo problems
On Tue, Apr 22, 2014 at 09:49:42PM -0400, Ted Unangst wrote:
> On Mon, Apr 14, 2014 at 12:12, Otto Moerbeek wrote:
> > On Sun, Apr 13, 2014 at 06:34:17PM -0400, Ted Unangst wrote:
> >
> >> I took another look at the way junk works in malloc, and there's a few
> >> improvements I'd like to make.
>
> >> I have't been able to build perl reliably with this diff, but haven't
> >> ruled out the 5.18 update either.
>
> This was just update/make -j 2 wonkiness. All good now.
>
> > - always junk half a page? Maybe that's a tad over done, athough it
> > might be relatively cheap since your writing the page anyway.
> >
> > - Chunks are now always completely junked on free, maybe a bit
> > overdone for malloc_junk = 1
>
> It doesn't seem any slower. I'd like to commit this, then see if
> anybody complains loud enough for me to hear them. :) hard to get
> consistent build times these days...
>
> > - if we are freeing a large region, it does not end up in the cache,
> > so junking (part of) it is a waste of effort. (That already is the
> > case for current, you are not introducing that).
>
> There's a few other things I'd like to change about the freelist. Next
> diff.
>
> > - What about 'j'? Do we want it to cancel a J or always disable
> > junking?
>
> I think disable entirely is the right answer.
>
> Here's the diff again. I was going to update it, but I don't see
> anything I want to change. I think it's fine as is for the first round.
> (The man page parts already snuck in.)
OK, but we should keep a close watch on performance issues.
-Otto
>
> Index: malloc.c
> ===
> RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
> retrieving revision 1.152
> diff -u -p -r1.152 malloc.c
> --- malloc.c 3 Apr 2014 16:18:11 - 1.152
> +++ malloc.c 13 Apr 2014 21:05:11 -
> @@ -167,7 +167,6 @@ struct malloc_readonly {
> int malloc_move;/* move allocations to end of page? */
> int malloc_realloc; /* always realloc? */
> int malloc_xmalloc; /* xmalloc behaviour? */
> - int malloc_zero;/* zero fill? */
> size_t malloc_guard; /* use guard pages after allocations? */
> u_int malloc_cache; /* free pages we cache */
> #ifdef MALLOC_STATS
> @@ -412,7 +411,7 @@ map(struct dir_info *d, size_t sz, int z
> d->free_regions_size -= psz;
> if (zero_fill)
> memset(p, 0, sz);
> - else if (mopts.malloc_junk &&
> + else if (mopts.malloc_junk == 2 &&
> mopts.malloc_freeunmap)
> memset(p, SOME_FREEJUNK, sz);
> return p;
> @@ -431,7 +430,7 @@ map(struct dir_info *d, size_t sz, int z
> d->free_regions_size -= psz;
> if (zero_fill)
> memset(p, 0, sz);
> - else if (mopts.malloc_junk && mopts.malloc_freeunmap)
> + else if (mopts.malloc_junk == 2 && mopts.malloc_freeunmap)
> memset(p, SOME_FREEJUNK, sz);
> return p;
> }
> @@ -461,6 +460,7 @@ omalloc_init(struct dir_info **dp)
>* Default options
>*/
> mopts.malloc_abort = 1;
> + mopts.malloc_junk = 1;
> mopts.malloc_move = 1;
> mopts.malloc_cache = MALLOC_DEFAULT_CACHE;
>
> @@ -534,7 +534,7 @@ omalloc_init(struct dir_info **dp)
> mopts.malloc_junk = 0;
> break;
> case 'J':
> - mopts.malloc_junk = 1;
> + mopts.malloc_junk = 2;
> break;
> case 'n':
> case 'N':
> @@ -557,7 +557,8 @@ omalloc_init(struct dir_info **dp)
> mopts.malloc_cache = MALLOC_DEFAULT_CACHE;
> break;
> case 'S':
> - mopts.malloc_freeunmap = mopts.malloc_junk = 1;
> + mopts.malloc_freeunmap = 1;
> + mopts.malloc_junk = 2;
> mopts.malloc_guard = MALLOC_PAGESIZE;
> mopts.malloc_cache = 0;
> break;
> @@ -573,12 +574,6 @@ omalloc_init(struct dir_info **dp)
> case 'X':
> mopts.malloc_xmalloc = 1;
> break;
> - case 'z':
> - mopts.malloc_zero = 0;
> - break;
> - case 'Z':
> - mopts.malloc_zero = 1;
> - break;
> default: {
>
