Re: apmd: -t: use strtonum()

2018-11-30 Thread Martijn van Duren
On 12/1/18 8:34 AM, Theo Buehler wrote:
> On Sat, Dec 01, 2018 at 08:01:00AM +0100, Martijn van Duren wrote:
>> On 12/1/18 1:44 AM, Klemens Nanni wrote:
> 
>>> # ./obj/apmd -dt -1
>>> apmd: too small seconds: -1
>>> # ./obj/apmd -dt 0
>>> apmd: too small seconds: 0
> 
> This looks odd, seconds aren't too big or too small.
> 
>> If you change the -t case to errc like the rest OK martijn@
> 
> I'm not sure the EINVAL error string adds valuable information.  I would
> prefer if all these used variants of the idiom suggested in the strtonum
> manual, something like:
> 
>   errx("number of seconds is %s: %s", errstr, optarg);
>   errx("battery percentage is %s: %s", errstr, optarg);
> 
That might be even better.



Re: apmd: -t: use strtonum()

2018-11-30 Thread Theo Buehler
On Sat, Dec 01, 2018 at 08:01:00AM +0100, Martijn van Duren wrote:
> On 12/1/18 1:44 AM, Klemens Nanni wrote:

> > # ./obj/apmd -dt -1
> > apmd: too small seconds: -1
> > # ./obj/apmd -dt 0
> > apmd: too small seconds: 0

This looks odd, seconds aren't too big or too small.

> If you change the -t case to errc like the rest OK martijn@

I'm not sure the EINVAL error string adds valuable information.  I would
prefer if all these used variants of the idiom suggested in the strtonum
manual, something like:

errx("number of seconds is %s: %s", errstr, optarg);
errx("battery percentage is %s: %s", errstr, optarg);



Re: apmd: -t: use strtonum()

2018-11-30 Thread Martijn van Duren



On 12/1/18 1:44 AM, Klemens Nanni wrote:
> Base 10 suffices, negative numbers should be invalid (not converted) and
> zero not treated specially:
> 
>   # apmd -dt -1
>   apmd: kevent loop: Invalid argument
>   # apmd -dt 0
>   usage: apmd [-AadHLs] [-f devname] [-S sockname] [-t seconds] [-Z 
> percent] [-z percent]
>   # apmd -dt 1
>   ^C
> 
>   # ./obj/apmd -dt -1
>   apmd: too small seconds: -1
>   # ./obj/apmd -dt 0
>   apmd: too small seconds: 0
>   # ./obj/apmd -dt 1
>   ^C
> 
> errstr is precise enough, so drop the "Invalid argument" by using just
> err() while here.

I wouldn't object to changing it to errx, but I reckon this is fine
as is. Changing it as you propose would be weird anyway:
$ ./obj/apmd -z -10
apmd: too small percentage: -10: Result too large

If you change the -t case to errc like the rest OK martijn@
> 
> Feedback? OK?
> 
> Index: apmd.c
> ===
> RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v
> retrieving revision 1.82
> diff -u -p -r1.82 apmd.c
> --- apmd.c30 Nov 2018 18:05:31 -  1.82
> +++ apmd.c1 Dec 2018 00:33:24 -
> @@ -392,9 +392,9 @@ main(int argc, char *argv[])
>   sockname = optarg;
>   break;
>   case 't':
> - ts.tv_sec = strtoul(optarg, NULL, 0);
> - if (ts.tv_sec == 0)
> - usage();
> + ts.tv_sec = strtonum(optarg, 1, LLONG_MAX, &errstr);
> + if (errstr != NULL)
> + err(1, "%s seconds: %s", errstr, optarg);
>   break;
>   case 's':   /* status only */
>   statonly = 1;
> @@ -422,15 +422,13 @@ main(int argc, char *argv[])
>   autoaction = AUTO_HIBERNATE;
>   autolimit = strtonum(optarg, 1, 100, &errstr);
>   if (errstr != NULL)
> - errc(1, EINVAL, "%s percentage: %s", errstr,
> - optarg);
> + err(1, "%s percentage: %s", errstr, optarg);
>   break;
>   case 'z':
>   autoaction = AUTO_SUSPEND;
>   autolimit = strtonum(optarg, 1, 100, &errstr);
>   if (errstr != NULL)
> - errc(1, EINVAL, "%s percentage: %s", errstr,
> - optarg);
> + err(1, "%s percentage: %s", errstr, optarg);
>   break;
>   case '?':
>   default:
> 



Fwd: uvm_fault: ip_ctloutput

2018-11-30 Thread Greg Steuck
The C reproducer panics the machine like a charm. Requires root.
https://syzkaller.appspot.com/x/repro.c?x=117e573340

-- Forwarded message -
From: syzbot 
Date: Fri, Nov 30, 2018 at 7:58 PM
Subject: Re: uvm_fault: ip_ctloutput
To: 


syzbot has found a reproducer for the following crash on:

HEAD commit:e9b93a3e5ebc Remove erroneous quote added in previous
git tree:   https://github.com/openbsd/src.git master
console output: https://syzkaller.appspot.com/x/log.txt?x=11a2362540
dashboard link: https://syzkaller.appspot.com/bug?extid=02168317bd0156c13b69
compiler:
syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=111b11a340
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=117e573340

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+02168317bd0156c13...@syzkaller.appspotmail.com

login: uvm_fault(0xff007f12bb58, 0xd0, 0, 1) -> e
kernel: page fault trap, code=0
Stopped at  ip_ctloutput+0x784: movq0xd0(%r14),%rbx
ddb>
ddb> set $lines = 0
ddb> show panic
kernel page fault
uvm_fault(0xff007f12bb58, 0xd0, 0, 1) -> e
ip_ctloutput(ff006e48e170,8000210c2e20,ff006e706788,8000210fa988,ff007f146c00)

at
ip_ctloutput+0x784
end trace frame: 0x8000210fa930, count: 0
ddb> trace
ip_ctloutput(ff006e48e170,8000210c2e20,ff006e706788,8000210fa988,ff007f146c00)

at
ip_ctloutput+0x784
sys_getsockopt(8000210faa10,8000210c2e20,8000210a5338) at
sys_getsockopt+0x13c
syscall(0) at syscall+0x3e4
Xsyscall(6,0,0,0,1,7f7f3a18) at Xsyscall+0x128
end of kernel
end trace frame: 0x7f7f39d0, count: -4
ddb> show registers
rdi0
rsi   0xff006e706788
rbp   0x8000210fa8d0
rbx0
rdx0
rcx  0x1
rax0
r80xff007f146c00
r9 0
r10   0xa28679f43345c2df
r11   0x8110e110rip_ctloutput
r12  0x1
r130
r140
r15   0xff007f146c00
rip   0x81a13b44ip_ctloutput+0x784
cs   0x8
rflags   0x10246__ALIGN_SIZE+0xf246
rsp   0x8000210fa8a0
ss  0x10
ip_ctloutput+0x784: movq0xd0(%r14),%rbx
ddb> show proc
PROC (syz-executor1283) pid=307178 stat=onproc
 flags process=2 proc=0
 pri=51, usrpri=51, nice=20
 forw=0x, list=0x8000210c3078,0x81e98cf0
 process=0x8000210a5338 user=0x8000210f5000,
vmspace=0xff007f12bb58
 estcpu=1, cpticks=1, pctcpu=0.0
 user=0, sys=1, intr=0
ddb> ps
PID TID   PPIDUID  S   FLAGS  WAIT  COMMAND
*22391  307178  19661  0  7 0x2syz-executor1283
  19661  340086  17670  0  30x10008a  pause ksh
  17670  326992  29604  0  30x92  selectsshd
  41270   33654  1  0  30x100083  ttyin getty
  29604  327245  1  0  30x80  selectsshd
  79075   90932  56293 73  20x100090syslogd
  56293  303628  1  0  30x100082  netio syslogd
  68459  425749  1 77  30x100090  poll  dhclient
  36911   58752  1  0  30x80  poll  dhclient
  56206  238502  0  0  2 0x14200zerothread
   5835  239343  0  0  3 0x14200  aiodoned  aiodoned
  38692  124704  0  0  3 0x14200  syncerupdate
  30045  377418  0  0  3 0x14200  cleaner   cleaner
   8830  232312  0  0  3 0x14200  reaperreaper
  36321  273872  0  0  3 0x14200  pgdaemon  pagedaemon
  27140  184915  0  0  3 0x14200  bored crynlk
  99803  446221  0  0  3 0x14200  bored crypto
  11482  154614  0  0  3  0x40014200  acpi0 acpi0
  50541  283257  0  0  3 0x14200  bored softnet
  80198  487934  0  0  3 0x14200  bored systqmp
  67536  180871  0  0  3 0x14200  bored systq
  44741  199952  0  0  3  0x40014200  bored softclock
  30804  187632  0  0  3  0x40014200idle0
  1   82730  0  0  30x82  wait  init
  0   0 -1  0  3 0x10200  scheduler swapper
ddb>

-- 
You received this message because you are subscribed to the Google Groups
"syzkaller-openbsd-bugs" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to syzkaller-openbsd-bugs+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/syzkaller-o

apmd: -t: use strtonum()

2018-11-30 Thread Klemens Nanni
Base 10 suffices, negative numbers should be invalid (not converted) and
zero not treated specially:

# apmd -dt -1
apmd: kevent loop: Invalid argument
# apmd -dt 0
usage: apmd [-AadHLs] [-f devname] [-S sockname] [-t seconds] [-Z 
percent] [-z percent]
# apmd -dt 1
^C

# ./obj/apmd -dt -1
apmd: too small seconds: -1
# ./obj/apmd -dt 0
apmd: too small seconds: 0
# ./obj/apmd -dt 1
^C

errstr is precise enough, so drop the "Invalid argument" by using just
err() while here.

Feedback? OK?

Index: apmd.c
===
RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v
retrieving revision 1.82
diff -u -p -r1.82 apmd.c
--- apmd.c  30 Nov 2018 18:05:31 -  1.82
+++ apmd.c  1 Dec 2018 00:33:24 -
@@ -392,9 +392,9 @@ main(int argc, char *argv[])
sockname = optarg;
break;
case 't':
-   ts.tv_sec = strtoul(optarg, NULL, 0);
-   if (ts.tv_sec == 0)
-   usage();
+   ts.tv_sec = strtonum(optarg, 1, LLONG_MAX, &errstr);
+   if (errstr != NULL)
+   err(1, "%s seconds: %s", errstr, optarg);
break;
case 's':   /* status only */
statonly = 1;
@@ -422,15 +422,13 @@ main(int argc, char *argv[])
autoaction = AUTO_HIBERNATE;
autolimit = strtonum(optarg, 1, 100, &errstr);
if (errstr != NULL)
-   errc(1, EINVAL, "%s percentage: %s", errstr,
-   optarg);
+   err(1, "%s percentage: %s", errstr, optarg);
break;
case 'z':
autoaction = AUTO_SUSPEND;
autolimit = strtonum(optarg, 1, 100, &errstr);
if (errstr != NULL)
-   errc(1, EINVAL, "%s percentage: %s", errstr,
-   optarg);
+   err(1, "%s percentage: %s", errstr, optarg);
break;
case '?':
default:



Re: pvclock(4)

2018-11-30 Thread johnw

johnw 於 2018-11-29 17:38 寫到:

Reyk Floeter 於 2018-11-29 14:09 寫到:


Do you have a full dmesg for me?


Yes, attached dmesg.log, thanks.


Hi, after disable pvclock, it can boot with new kernel again, thanks.



--
Key fingerprint: CDB3 6C62 254B C088 1E5D DD32 182C 97DB CF2C 80AC
OpenBSD 6.4-current (GENERIC.MP) #447: Sun Nov 18 17:25:58 MST 2018
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 519954432 (495MB)
avail mem = 494972928 (472MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xf5b80 (10 entries)
bios0: vendor SeaBIOS version "1.11.1-1" date 04/01/2014
bios0: QEMU Standard PC (i440FX + PIIX, 1996)
acpi0 at bios0: rev 0
acpi0: sleep states S3 S4 S5
acpi0: tables DSDT FACP APIC HPET
acpi0: wakeup devices
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)2 Duo CPU E8400 @ 3.00GHz, 105.29 MHz, 06-17-0a
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,SSE3,SSSE3,CX16,SSE4.1,x2APIC,DEADLINE,XSAVE,HV,NXE,LONG,LAHF,PERF,ARAT,MELTDOWN
cpu0: 64KB 64b/line 2-way I-cache, 64KB 64b/line 2-way D-cache, 512KB 64b/line 
16-way L2 cache
cpu0: ITLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
cpu0: DTLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 1009MHz
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Intel(R) Core(TM)2 Duo CPU E8400 @ 3.00GHz, 165.45 MHz, 06-17-0a
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,SSE3,SSSE3,CX16,SSE4.1,x2APIC,DEADLINE,XSAVE,HV,NXE,LONG,LAHF,PERF,ARAT,MELTDOWN
cpu1: 64KB 64b/line 2-way I-cache, 64KB 64b/line 2-way D-cache, 512KB 64b/line 
16-way L2 cache
cpu1: ITLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
cpu1: DTLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
cpu1: smt 0, core 0, package 1
ioapic0 at mainbus0: apid 0 pa 0xfec0, version 20, 24 pins
acpihpet0 at acpi0: 1 Hz
acpiprt0 at acpi0: bus 0 (PCI0)
acpicpu0 at acpi0: C1(@1 halt!)
acpicpu1 at acpi0: C1(@1 halt!)
"ACPI0006" at acpi0 not configured
acpipci0 at acpi0 PCI0: _OSC failed
acpicmos0 at acpi0
"PNP0A06" at acpi0 not configured
"PNP0A06" at acpi0 not configured
"PNP0A06" at acpi0 not configured
"QEMU0002" at acpi0 not configured
"ACPI0010" at acpi0 not configured
pvbus0 at mainbus0: KVM
pci0 at mainbus0 bus 0
pchb0 at pci0 dev 0 function 0 "Intel 82441FX" rev 0x02
pcib0 at pci0 dev 1 function 0 "Intel 82371SB ISA" rev 0x00
pciide0 at pci0 dev 1 function 1 "Intel 82371SB IDE" rev 0x00: DMA, channel 0 
wired to compatibility, channel 1 wired to compatibility
pciide0: channel 0 disabled (no drives)
atapiscsi0 at pciide0 channel 1 drive 0
scsibus1 at atapiscsi0: 2 targets
cd0 at scsibus1 targ 0 lun 0:  ATAPI 5/cdrom removable
cd0(pciide0:1:0): using PIO mode 4, DMA mode 2
piixpm0 at pci0 dev 1 function 3 "Intel 82371AB Power" rev 0x03: apic 0 int 9
iic0 at piixpm0
vga1 at pci0 dev 2 function 0 "Bochs VGA" rev 0x02
wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation)
wsdisplay0: screen 1-5 added (80x25, vt100 emulation)
virtio0 at pci0 dev 3 function 0 "Qumranet Virtio Network" rev 0x00
vio0 at virtio0: address aa:aa:aa:aa:aa:01
virtio0: msix shared
virtio1 at pci0 dev 4 function 0 "Qumranet Virtio Storage" rev 0x00
vioblk0 at virtio1
scsibus2 at vioblk0: 2 targets
sd0 at scsibus2 targ 0 lun 0:  SCSI3 0/direct fixed
sd0: 20480MB, 512 bytes/sector, 41943040 sectors
virtio1: msix shared
isa0 at pcib0
isadma0 at isa0
fdc0 at isa0 port 0x3f0/6 irq 6 drq 2
com0 at isa0 port 0x3f8/8 irq 4: ns16550a, 16 byte fifo
pckbc0 at isa0 port 0x60/5 irq 1 irq 12
pckbd0 at pckbc0 (kbd slot)
wskbd0 at pckbd0: console keyboard, using wsdisplay0
pms0 at pckbc0 (aux slot)
wsmouse0 at pms0 mux 0
pcppi0 at isa0 port 0x61
spkr0 at pcppi0
lpt0 at isa0 port 0x378/4 irq 7
vscsi0 at root
scsibus3 at vscsi0: 256 targets
softraid0 at root
scsibus4 at softraid0: 256 targets
sd1 at scsibus4 targ 1 lun 0:  SCSI2 0/direct fixed
sd1: 20473MB, 512 bytes/sector, 41929058 sectors
root on sd1a (fa1d7de7776af250.a) swap on sd1b dump on sd1b
fd0 at fdc0 drive 1: density unknown
syncing disks... done
rebooting...
OpenBSD 6.4-current (GENERIC.MP) #447: Sun Nov 18 17:25:58 MST 2018
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 519954432 (495MB)
avail mem = 494972928 (472MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xf5b80 (10 entries)
bios0: vendor SeaBIOS version "1.11.1-1" date 04/01/2014
bios0: QEMU Standard PC (i440FX + PIIX, 1996)
acpi0 at bios0: rev 0
acpi0: sleep states S3 S4 S5
acpi0: tables DSDT FACP APIC HPET
acpi0: wakeup devices
acpitimer0 at a

Re: apmd debug

2018-11-30 Thread Klemens Nanni
On Fri, Nov 30, 2018 at 01:24:27PM -0500, Ted Unangst wrote:
> Developers who shall remain anonymous were confused by the behavior of apmd -d
> because the behavior of apmd -d is confusing. It doesn't do anything like any
> other daemon in the system when running with -d.
:-)

> This introduces a logmsg function to replace syslog, which will either syslog
> or printf depending on debug. I think this makes debugging much easier.
Yes it does.

Code looks good but the manual bits are missing.



Re: free(9) sizes for sysv_msg

2018-11-30 Thread Alexandre Ratchov
On Fri, Nov 30, 2018 at 04:02:07PM -0200, Martin Pieuchot wrote:
> On 29/11/18(Thu) 22:42, Alexandre Ratchov wrote:
> > On Thu, Nov 29, 2018 at 04:51:19PM -0200, Martin Pieuchot wrote:
> > > Trivial one, ok?
> > > 
> > > Index: kern/sysv_msg.c
> > > ===
> > > RCS file: /cvs/src/sys/kern/sysv_msg.c,v
> > > retrieving revision 1.33
> > > diff -u -p -r1.33 sysv_msg.c
> > > --- kern/sysv_msg.c   15 Sep 2016 02:00:16 -  1.33
> > > +++ kern/sysv_msg.c   29 Nov 2018 18:47:05 -
> > > @@ -699,7 +699,7 @@ sysctl_sysvmsg(int *name, u_int namelen,
> > >   msginfo.msgmni * sizeof(struct msqid_ds);
> > >  
> > 
> > infolen is calculated twice; the first infolen calculation is used as
> > argument to malloc(). Your diff makes the second one the size argument
> > to free(), which doesn't seem correct.
> 
> Thanks for pointing that out.  Revised diff adding & using a different
> variable.
> 

ok ratchov



Re: Add new PCI product IDs

2018-11-30 Thread Mike Larkin
On Wed, Nov 07, 2018 at 11:33:55AM -0800, Peter Ezetta wrote:
> Ping
> 

Thanks, committed. I changed the first name a bit to remain more inline
with our current naming scheme.

-ml

> On Thu, Oct 25, 2018 at 2:46 PM Peter Ezetta  wrote:
> 
> > Hello,
> >
> > Diff below adds product IDs for the Nvidia Quadro M1200 Mobile graphics
> > card and the Intel Xeon E3-1200 v6 7th gen Host Bridge (for mobile).
> >
> > Index: pcidevs
> > ===
> > RCS file: /cvs/src/sys/dev/pci/pcidevs,v
> > retrieving revision 1.1863
> > diff -u -p -r1.1863 pcidevs
> > --- pcidevs 22 Oct 2018 05:06:32 -  1.1863
> > +++ pcidevs 25 Oct 2018 21:36:12 -
> > @@ -4718,6 +4718,7 @@ product INTEL CORE7G_U_HB 0x5904  Core 7G
> >  product INTEL CORE7G_U_GT1 0x5906  HD Graphics 610
> >  product INTEL CORE7G_Y_HB  0x590c  Core 7G Host
> >  product INTEL CORE7G_Y_GT1 0x590e  HD Graphics
> > +product INTEL XEONE3_1200V6M_HB0x5910  Xeon E3-1200 v6/7 Host
> >  product INTEL CORE_GMM_2   0x5911  Core GMM
> >  product INTEL CORE7G_S_GT2 0x5912  HD Graphics 630
> >  product INTEL CORE8G_U_HB  0x5914  Core 8G Host
> > @@ -6529,6 +6530,7 @@ product NVIDIA GEFORCE940MX   0x134d  GeFor
> >  product NVIDIA GEFORCEGTX750TI 0x1380  GeForce GTX 750 Ti
> >  product NVIDIA GEFORCEGTX750   0x1381  GeForce GTX 750
> >  product NVIDIA GEFORCEGTX745   0x1382  GeForce GTX 745
> > +product NVIDIA QUADROM1200 0x13b6  Quadro M1200
> >
> >  /* Oak Technologies products */
> >  product OAKTECH OTI10070x0107  OTI107
> >



apmd debug

2018-11-30 Thread Ted Unangst
Developers who shall remain anonymous were confused by the behavior of apmd -d
because the behavior of apmd -d is confusing. It doesn't do anything like any
other daemon in the system when running with -d.

This introduces a logmsg function to replace syslog, which will either syslog
or printf depending on debug. I think this makes debugging much easier.


Index: apmd.c
===
RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v
retrieving revision 1.82
diff -u -p -r1.82 apmd.c
--- apmd.c  30 Nov 2018 18:05:31 -  1.82
+++ apmd.c  30 Nov 2018 18:21:02 -
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -92,6 +93,21 @@ sigexit(int signo)
 }
 
 void
+logmsg(int prio, const char *msg, ...)
+{
+   va_list ap;
+
+   va_start(ap, msg);
+   if (debug) {
+   vfprintf(stdout, msg, ap);
+   fprintf(stdout, "\n");
+   } else {
+   vsyslog(prio, msg, ap);
+   }
+   va_end(ap);
+}
+
+void
 usage(void)
 {
fprintf(stderr,
@@ -123,7 +139,8 @@ void
 set_driver_messages(int fd, int mode)
 {
if (ioctl(fd, APM_IOC_PRN_CTL, &mode) == -1)
-   syslog(LOG_DEBUG, "can't disable driver messages, error: %m");
+   logmsg(LOG_DEBUG, "can't disable driver messages, error: %s",
+   strerror(errno));
 }
 
 int
@@ -168,7 +185,7 @@ power_status(int fd, int force, struct a
 #else
if ((int)bstate.minutes_left > 0)
 #endif
-   syslog(LOG_NOTICE, "battery status: %s. "
+   logmsg(LOG_NOTICE, "battery status: %s. "
"external power status: %s. "
"estimated battery life %d%% (%u minutes)",
battstate(bstate.battery_state),
@@ -176,7 +193,7 @@ power_status(int fd, int force, struct a
bstate.battery_life,
bstate.minutes_left);
else
-   syslog(LOG_NOTICE, "battery status: %s. "
+   logmsg(LOG_NOTICE, "battery status: %s. "
"external power status: %s. "
"estimated battery life %d%%",
battstate(bstate.battery_state),
@@ -187,7 +204,7 @@ power_status(int fd, int force, struct a
if (pinfo)
*pinfo = bstate;
} else
-   syslog(LOG_ERR, "cannot fetch power status: %m");
+   logmsg(LOG_ERR, "cannot fetch power status: %m");
 
return acon;
 }
@@ -248,13 +265,13 @@ handle_client(int sock_fd, int ctl_fd)
fromlen = sizeof(from);
cli_fd = accept(sock_fd, (struct sockaddr *)&from, &fromlen);
if (cli_fd == -1) {
-   syslog(LOG_INFO, "client accept failure: %m");
+   logmsg(LOG_INFO, "client accept failure: %s", strerror(errno));
return NORMAL;
}
 
if (recv(cli_fd, &cmd, sizeof(cmd), 0) != sizeof(cmd)) {
(void) close(cli_fd);
-   syslog(LOG_INFO, "client size botch");
+   logmsg(LOG_INFO, "client size botch");
return NORMAL;
}
 
@@ -278,20 +295,20 @@ handle_client(int sock_fd, int ctl_fd)
case SETPERF_LOW:
doperf = PERF_MANUAL;
reply.newstate = NORMAL;
-   syslog(LOG_NOTICE, "setting hw.perfpolicy to low");
+   logmsg(LOG_NOTICE, "setting hw.perfpolicy to low");
setperfpolicy("low");
break;
case SETPERF_HIGH:
doperf = PERF_MANUAL;
reply.newstate = NORMAL;
-   syslog(LOG_NOTICE, "setting hw.perfpolicy to high");
+   logmsg(LOG_NOTICE, "setting hw.perfpolicy to high");
setperfpolicy("high");
break;
case SETPERF_AUTO:
case SETPERF_COOL:
doperf = PERF_AUTO;
reply.newstate = NORMAL;
-   syslog(LOG_NOTICE, "setting hw.perfpolicy to auto");
+   logmsg(LOG_NOTICE, "setting hw.perfpolicy to auto");
setperfpolicy("auto");
break;
default:
@@ -300,13 +317,13 @@ handle_client(int sock_fd, int ctl_fd)
}
 
if (sysctl(cpuspeed_mib, 2, &cpuspeed, &cpuspeed_sz, NULL, 0) < 0)
-   syslog(LOG_INFO, "cannot read hw.cpuspeed");
+   logmsg(LOG_INFO, "cannot read hw.cpuspeed");
 
reply.cpuspeed = cpuspeed;
reply.perfmode = doperf;
reply.vno = APMD_VNO;
if (send(cli_fd, &reply, sizeof(reply), 0) != sizeof(reply))
-   syslog(LOG_INFO, "reply to client botched");
+   logmsg(LOG_INFO, "reply to client botched");
   

Re: fuse_opt.3 patch

2018-11-30 Thread Martin Pieuchot
On 30/11/18(Fri) 07:07, Edgar Pettijohn wrote:
> Here is a diff to remove the dubious example and fix one gramatical issue. 

Thanks.  Don't forget to use "mandoc -Tlint" to spot potentials errors
after a change.  In this case you forgot to remove an ".Ed".

Keep the good work.  All fuse manuals need reviews & improvements :)



Re: free(9) sizes for sysv_msg

2018-11-30 Thread Martin Pieuchot
On 29/11/18(Thu) 22:42, Alexandre Ratchov wrote:
> On Thu, Nov 29, 2018 at 04:51:19PM -0200, Martin Pieuchot wrote:
> > Trivial one, ok?
> > 
> > Index: kern/sysv_msg.c
> > ===
> > RCS file: /cvs/src/sys/kern/sysv_msg.c,v
> > retrieving revision 1.33
> > diff -u -p -r1.33 sysv_msg.c
> > --- kern/sysv_msg.c 15 Sep 2016 02:00:16 -  1.33
> > +++ kern/sysv_msg.c 29 Nov 2018 18:47:05 -
> > @@ -699,7 +699,7 @@ sysctl_sysvmsg(int *name, u_int namelen,
> > msginfo.msgmni * sizeof(struct msqid_ds);
> >  
> 
> infolen is calculated twice; the first infolen calculation is used as
> argument to malloc(). Your diff makes the second one the size argument
> to free(), which doesn't seem correct.

Thanks for pointing that out.  Revised diff adding & using a different
variable.

Index: kern/sysv_msg.c
===
RCS file: /cvs/src/sys/kern/sysv_msg.c,v
retrieving revision 1.33
diff -u -p -r1.33 sysv_msg.c
--- kern/sysv_msg.c 15 Sep 2016 02:00:16 -  1.33
+++ kern/sysv_msg.c 30 Nov 2018 17:59:43 -
@@ -658,7 +658,7 @@ sysctl_sysvmsg(int *name, u_int namelen,
 {
struct msg_sysctl_info *info;
struct que *que;
-   size_t infolen;
+   size_t infolen, infolen0;
int error;
 
switch (*name) {
@@ -675,10 +675,10 @@ sysctl_sysvmsg(int *name, u_int namelen,
 * message queues; for now, emulate this behavior
 * until a more thorough fix can be made.
 */
-   infolen = sizeof(msginfo) +
+   infolen0 = sizeof(msginfo) +
msginfo.msgmni * sizeof(struct msqid_ds);
if (where == NULL) {
-   *sizep = infolen;
+   *sizep = infolen0;
return (0);
}
 
@@ -692,14 +692,14 @@ sysctl_sysvmsg(int *name, u_int namelen,
if (*sizep == sizeof(struct msginfo))
return (copyout(&msginfo, where, sizeof(msginfo)));
 
-   info = malloc(infolen, M_TEMP, M_WAIT|M_ZERO);
+   info = malloc(infolen0, M_TEMP, M_WAIT|M_ZERO);
 
/* if the malloc slept, this may have changed */
infolen = sizeof(msginfo) +
msginfo.msgmni * sizeof(struct msqid_ds);
 
if (*sizep < infolen) {
-   free(info, M_TEMP, 0);
+   free(info, M_TEMP, infolen0);
return (ENOMEM);
}
 
@@ -716,7 +716,7 @@ sysctl_sysvmsg(int *name, u_int namelen,
 
error = copyout(info, where, infolen);
 
-   free(info, M_TEMP, 0);
+   free(info, M_TEMP, infolen0);
 
return (error);
 



Re: split ether_output up into resolution, encapsulation, and output

2018-11-30 Thread Claudio Jeker
On Fri, Nov 30, 2018 at 02:04:40PM -0200, Martin Pieuchot wrote:
> On 30/11/18(Fri) 12:35, David Gwynne wrote:
> > On Fri, Nov 30, 2018 at 12:21:11PM +1000, David Gwynne wrote:
> > > i have a plan to allow virtual interfaces (eg, vlan, etherip, etc) to
> > > provide their own output functions so they can bypass the ifq machinery
> > > and push the packet onto the underlying layer directly.
> > > 
> > > they'll still need to get an ethernet header though. vlan needs to get
> > > the ethernet header and put the vlan shim into it, therefore
> > > ether_resolve is exposed. etherip doesnt need a shim, it just wants
> > > ethernet encapsulating the payload before adding its own headers to the
> > > packet, therefore there is ether_encap.
> > > 
> > > does this make sense?
> > > 
> > > ok?
> > 
> > this shows vlan and etherip using the new functions.
> 
> You're still using ifq_enqueue() and ifq_dequeue(). So, I'm not sure to
> understand what is the gain of this change.

If I understood the change correctly it will bypass queueing when no HFSC
queueing is configured on that interface. If there is no traffic shaping
in use the ifp_output function will directly call the next ifp_output
function or in the case of etherip / gre /gif it will call ip_send.

IMO this is a step in the right direction.
 
> > Index: net/if_etherip.c
> > ===
> > RCS file: /cvs/src/sys/net/if_etherip.c,v
> > retrieving revision 1.40
> > diff -u -p -r1.40 if_etherip.c
> > --- net/if_etherip.c12 Nov 2018 23:57:06 -  1.40
> > +++ net/if_etherip.c30 Nov 2018 02:24:09 -
> > @@ -102,7 +102,10 @@ void etheripattach(int);
> >  int etherip_clone_create(struct if_clone *, int);
> >  int etherip_clone_destroy(struct ifnet *);
> >  int etherip_ioctl(struct ifnet *, u_long, caddr_t);
> > -void etherip_start(struct ifnet *);
> > +int etherip_output(struct ifnet *, struct mbuf *, struct sockaddr *,
> > +struct rtentry *rt);
> > +void etherip_start(struct ifqueue *);
> > +void etherip_send(struct ifnet *, struct mbuf *);
> >  int etherip_media_change(struct ifnet *);
> >  void etherip_media_status(struct ifnet *, struct ifmediareq *);
> >  int etherip_set_tunnel(struct etherip_softc *, struct if_laddrreq *);
> > @@ -144,9 +147,10 @@ etherip_clone_create(struct if_clone *if
> > ifp->if_softc = sc;
> > ifp->if_hardmtu = ETHER_MAX_HARDMTU_LEN;
> > ifp->if_ioctl = etherip_ioctl;
> > -   ifp->if_start = etherip_start;
> > +   ifp->if_output = etherip_output;
> > +   ifp->if_qstart = etherip_start;
> > ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
> > -   ifp->if_xflags = IFXF_CLONED;
> > +   ifp->if_xflags = IFXF_MPSAFE | IFXF_CLONED;
> > IFQ_SET_MAXLEN(&ifp->if_snd, IFQ_MAXLEN);
> > ifp->if_capabilities = IFCAP_VLAN_MTU;
> > ether_fakeaddr(ifp);
> > @@ -201,40 +205,63 @@ etherip_media_status(struct ifnet *ifp, 
> >  }
> >  
> >  void
> > -etherip_start(struct ifnet *ifp)
> > +etherip_send(struct ifnet *ifp, struct mbuf *m)
> >  {
> > struct etherip_softc *sc = ifp->if_softc;
> > -   struct mbuf *m;
> > int error;
> > -#if NBPFILTER > 0
> > -   caddr_t if_bpf;
> > -#endif
> >  
> > -   while ((m = ifq_dequeue(&ifp->if_snd)) != NULL) {
> >  #if NBPFILTER > 0
> > -   if_bpf = ifp->if_bpf;
> > -   if (if_bpf)
> > -   bpf_mtap_ether(if_bpf, m, BPF_DIRECTION_OUT);
> > +   caddr_t if_bpf = ifp->if_bpf;
> > +   if (if_bpf)
> > +   bpf_mtap_ether(if_bpf, m, BPF_DIRECTION_OUT);
> >  #endif
> >  
> > -   switch (sc->sc_tunnel.t_af) {
> > -   case AF_INET:
> > -   error = ip_etherip_output(ifp, m);
> > -   break;
> > +   switch (sc->sc_tunnel.t_af) {
> > +   case AF_INET:
> > +   error = ip_etherip_output(ifp, m);
> > +   break;
> >  #ifdef INET6
> > -   case AF_INET6:
> > -   error = ip6_etherip_output(ifp, m);
> > -   break;
> > +   case AF_INET6:
> > +   error = ip6_etherip_output(ifp, m);
> > +   break;
> >  #endif
> > -   default:
> > -   /* unhandled_af(sc->sc_tunnel.t_af); */
> > -   m_freem(m);
> > -   continue;
> > -   }
> > +   default:
> > +   /* unhandled_af(sc->sc_tunnel.t_af); */
> > +   m_freem(m);
> > +   error = ENETDOWN;
> > +   break;
> > +   }
> >  
> > -   if (error)
> > -   ifp->if_oerrors++;
> > +   if (error)
> > +   ifp->if_oerrors++;
> > +}
> > +
> > +void
> > +etherip_start(struct ifqueue *ifq)
> > +{
> > +   struct ifnet *ifp = ifq->ifq_if;
> > +   struct mbuf *m;
> > +
> > +   while ((m = ifq_dequeue(ifq)) != NULL)
> > +   etherip_send(ifp, m);
> > +}
> > +
> > +int
> > +etherip_output(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst,
> > +struct rtentry *rt)
> > +{
> > +   int error;
> > +
> > +  

Re: split ether_output up into resolution, encapsulation, and output

2018-11-30 Thread Martin Pieuchot
On 30/11/18(Fri) 12:35, David Gwynne wrote:
> On Fri, Nov 30, 2018 at 12:21:11PM +1000, David Gwynne wrote:
> > i have a plan to allow virtual interfaces (eg, vlan, etherip, etc) to
> > provide their own output functions so they can bypass the ifq machinery
> > and push the packet onto the underlying layer directly.
> > 
> > they'll still need to get an ethernet header though. vlan needs to get
> > the ethernet header and put the vlan shim into it, therefore
> > ether_resolve is exposed. etherip doesnt need a shim, it just wants
> > ethernet encapsulating the payload before adding its own headers to the
> > packet, therefore there is ether_encap.
> > 
> > does this make sense?
> > 
> > ok?
> 
> this shows vlan and etherip using the new functions.

You're still using ifq_enqueue() and ifq_dequeue(). So, I'm not sure to
understand what is the gain of this change.

> Index: net/if_etherip.c
> ===
> RCS file: /cvs/src/sys/net/if_etherip.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 if_etherip.c
> --- net/if_etherip.c  12 Nov 2018 23:57:06 -  1.40
> +++ net/if_etherip.c  30 Nov 2018 02:24:09 -
> @@ -102,7 +102,10 @@ void etheripattach(int);
>  int etherip_clone_create(struct if_clone *, int);
>  int etherip_clone_destroy(struct ifnet *);
>  int etherip_ioctl(struct ifnet *, u_long, caddr_t);
> -void etherip_start(struct ifnet *);
> +int etherip_output(struct ifnet *, struct mbuf *, struct sockaddr *,
> +struct rtentry *rt);
> +void etherip_start(struct ifqueue *);
> +void etherip_send(struct ifnet *, struct mbuf *);
>  int etherip_media_change(struct ifnet *);
>  void etherip_media_status(struct ifnet *, struct ifmediareq *);
>  int etherip_set_tunnel(struct etherip_softc *, struct if_laddrreq *);
> @@ -144,9 +147,10 @@ etherip_clone_create(struct if_clone *if
>   ifp->if_softc = sc;
>   ifp->if_hardmtu = ETHER_MAX_HARDMTU_LEN;
>   ifp->if_ioctl = etherip_ioctl;
> - ifp->if_start = etherip_start;
> + ifp->if_output = etherip_output;
> + ifp->if_qstart = etherip_start;
>   ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
> - ifp->if_xflags = IFXF_CLONED;
> + ifp->if_xflags = IFXF_MPSAFE | IFXF_CLONED;
>   IFQ_SET_MAXLEN(&ifp->if_snd, IFQ_MAXLEN);
>   ifp->if_capabilities = IFCAP_VLAN_MTU;
>   ether_fakeaddr(ifp);
> @@ -201,40 +205,63 @@ etherip_media_status(struct ifnet *ifp, 
>  }
>  
>  void
> -etherip_start(struct ifnet *ifp)
> +etherip_send(struct ifnet *ifp, struct mbuf *m)
>  {
>   struct etherip_softc *sc = ifp->if_softc;
> - struct mbuf *m;
>   int error;
> -#if NBPFILTER > 0
> - caddr_t if_bpf;
> -#endif
>  
> - while ((m = ifq_dequeue(&ifp->if_snd)) != NULL) {
>  #if NBPFILTER > 0
> - if_bpf = ifp->if_bpf;
> - if (if_bpf)
> - bpf_mtap_ether(if_bpf, m, BPF_DIRECTION_OUT);
> + caddr_t if_bpf = ifp->if_bpf;
> + if (if_bpf)
> + bpf_mtap_ether(if_bpf, m, BPF_DIRECTION_OUT);
>  #endif
>  
> - switch (sc->sc_tunnel.t_af) {
> - case AF_INET:
> - error = ip_etherip_output(ifp, m);
> - break;
> + switch (sc->sc_tunnel.t_af) {
> + case AF_INET:
> + error = ip_etherip_output(ifp, m);
> + break;
>  #ifdef INET6
> - case AF_INET6:
> - error = ip6_etherip_output(ifp, m);
> - break;
> + case AF_INET6:
> + error = ip6_etherip_output(ifp, m);
> + break;
>  #endif
> - default:
> - /* unhandled_af(sc->sc_tunnel.t_af); */
> - m_freem(m);
> - continue;
> - }
> + default:
> + /* unhandled_af(sc->sc_tunnel.t_af); */
> + m_freem(m);
> + error = ENETDOWN;
> + break;
> + }
>  
> - if (error)
> - ifp->if_oerrors++;
> + if (error)
> + ifp->if_oerrors++;
> +}
> +
> +void
> +etherip_start(struct ifqueue *ifq)
> +{
> + struct ifnet *ifp = ifq->ifq_if;
> + struct mbuf *m;
> +
> + while ((m = ifq_dequeue(ifq)) != NULL)
> + etherip_send(ifp, m);
> +}
> +
> +int
> +etherip_output(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst,
> +struct rtentry *rt)
> +{
> + int error;
> +
> + m = ether_encap(ifp, m, dst, rt, &error);
> + if (m == NULL)
> + return (error);
> +
> + if (ifq_is_priq(&ifp->if_snd)) {
> + etherip_send(ifp, m);
> + return (0);
>   }
> +
> + return (ifq_enqueue(&ifp->if_snd, m));
>  }
>  
>  int
> Index: net/if_ethersubr.c
> ===
> RCS file: /cvs/src/sys/net/if_ethersubr.c,v
> retrieving revision 1.253
> diff -u -p -r1.253 if_ethersubr.c
> --- net/if_ethersubr.c13 Mar 2018 01:31:48

Re: athn(4) hostap: mem leak

2018-11-30 Thread Alexandre Ratchov
On Fri, Nov 30, 2018 at 01:49:56PM +0100, Benjamin Baier wrote:
> Hi
> 
> There is a leak of *arg in 
> dev/usb/if_athn_usb.c:athn_usb_newauth() line 1263
> since Rev. 1.49
> Because athn_usb_do_async() memcpy's the argument anyway.
> 
> Found with llvm/scan-build.
> 
> Instead of adding free(arg) I opted to make this function
> more like the other ones which call athn_usb_do_async.
> 

Hi,

AFAICS, athn_usb_do_async() will schedule a call to
athn_usb_newauth_cb(), which will use arg after the functin has
returned. The arg memory location must stay valid after return from
athn_usb_newauth(). So we can neither use free() nor a local variable.

The athn_usb_newauth_cb() callback calls free(), so there's no memory
leak unless the callback is cancelled. I don't know it can be
cancelled, I see no code doing so.

> Only compile tested... looking for tests.
> 
> Greetings Ben
> 
> Index: if_athn_usb.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_athn_usb.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 if_athn_usb.c
> --- if_athn_usb.c 6 Sep 2018 11:50:54 -   1.51
> +++ if_athn_usb.c 29 Nov 2018 18:33:40 -
> @@ -1231,7 +1231,7 @@ athn_usb_newauth(struct ieee80211com *ic
>   struct ifnet *ifp = &ic->ic_if;
>   struct athn_node *an = (struct athn_node *)ni;
>   int nsta;
> - struct athn_usb_newauth_cb_arg *arg;
> + struct athn_usb_newauth_cb_arg arg;
>  
>   if (ic->ic_opmode != IEEE80211_M_HOSTAP)
>   return 0;
> @@ -1254,12 +1254,9 @@ athn_usb_newauth(struct ieee80211com *ic
>* In a process context, try to add this node to the
>* firmware table and confirm the AUTH request.
>*/
> - arg = malloc(sizeof(*arg), M_DEVBUF, M_NOWAIT);
> - if (arg == NULL)
> - return ENOMEM;
> - arg->ni = ieee80211_ref_node(ni);
> - arg->seq = seq;
> - athn_usb_do_async(usc, athn_usb_newauth_cb, arg, sizeof(*arg));
> + arg.ni = ieee80211_ref_node(ni);
> + arg.seq = seq;
> + athn_usb_do_async(usc, athn_usb_newauth_cb, &arg, sizeof(arg));
>   return EBUSY;
>  #else
>   return 0;
> 

-- 



Re: httpd server configuration evaluation bug

2018-11-30 Thread Tracey Emery
On Fri, Nov 30, 2018 at 10:16:45AM +0100, Florian Obser wrote:
> On Fri, Oct 26, 2018 at 03:08:11PM -0600, Tracey Emery wrote:
> > On Mon, Jul 30, 2018 at 10:24:03AM -0600, Base Pr1me wrote:
> > > Sorry, this time with the correct diff.
> > > 
> > > On 7/25/18 4:15 PM, Base Pr1me wrote:
> > > > Hi,
> > > > 
> > > > I discovered that the wrong server configuration is evaluated in the
> > > > server_read_http function. Only the first server in httpd.conf is 
> > > > checked. For
> > > > example, I have five servers setup in httpd.conf and the third server 
> > > > is the
> > > > only one with connection { max request body } set, because I desire 
> > > > it to
> > > > accept larger uploads than the other servers. When the upload is 
> > > > initiated,
> > > > server one dictates the max request body size, globally.
> > > > 
> > > > The attached diff moves the queue loop out of the server_response 
> > > > function in to
> > > > its own function, as to not duplicate code.
> > > > 
> > > > I don't know if this is the only place the wrong information is 
> > > > evaluated. Also,
> > > > I'm not sure this is the best method to fix the problem, but it should 
> > > > point the
> > > > powers that be in the right direction.
> > > > 
> > > > Thanks,
> > > > 
> > > > Tracey
> > > > 
> > > 
> > 
> > Hello,
> > 
> > I reworked the last sent diff. I missed fixing up the hostname. This was 
> > causing
> > an incorrect 301 on urls not containing an ending slash. I also moved the
> > srv_conf assignment into the new function.
> > 
> > Again, this is to use the correct server config information from the queue 
> > for
> > server_read_http and remove code duplication.
> > 
> > If anyone is willing to look at this and make suggestions, that'd be great. 
> > If
> > not, that'd be great too! LOL. Have a great weekend.
> > 
> > Thanks,
> > Tracey
> 
> (moving back from bugs@)
> 
> Sorry for slacking of on this for so long and thanks for prodding
> patiently :)
> 
> The issue is that in server_read_http we haven't found the correct
> virtual server & location yet.
> clt->clt_srv_conf contains the server_config struct for the listening IP.
> 
> But there is no need to check maxrequestbody just yet, we can do it in
> server_response() where we do know the virtual host.
> 
> Tracey, can you please test this, it should solve your problem.
> 
> OK?
> 
> diff --git server_http.c server_http.c
> index e05cec56dfc..52698a66b2e 100644
> --- server_http.c
> +++ server_http.c
> @@ -198,7 +198,6 @@ void
>  server_read_http(struct bufferevent *bev, void *arg)
>  {
>   struct client   *clt = arg;
> - struct server_config*srv_conf = clt->clt_srv_conf;
>   struct http_descriptor  *desc = clt->clt_descreq;
>   struct evbuffer *src = EVBUFFER_INPUT(bev);
>   char*line = NULL, *key, *value;
> @@ -357,11 +356,6 @@ server_read_http(struct bufferevent *bev, void *arg)
>   server_abort_http(clt, 500, errstr);
>   goto abort;
>   }
> - if ((size_t)clt->clt_toread >
> - srv_conf->maxrequestbody) {
> - server_abort_http(clt, 413, NULL);
> - goto abort;
> - }
>   }
>  
>   if (strcasecmp("Transfer-Encoding", key) == 0 &&
> @@ -1334,6 +1328,12 @@ server_response(struct httpd *httpd, struct client 
> *clt)
>   srv_conf = server_getlocation(clt, desc->http_path);
>   }
>  
> + if (clt->clt_toread > 0 && (size_t)clt->clt_toread >
> + srv_conf->maxrequestbody) {
> + server_abort_http(clt, 413, NULL);
> + return (-1);
> + }
> +
>   if (srv_conf->flags & SRVFLAG_BLOCK) {
>   server_abort_http(clt, srv_conf->return_code,
>   srv_conf->return_uri);
> 
> 
> 
> -- 
> I'm not entirely sure you are real.

Thank you, sir. That does, indeed, fix the maxrequestbody issue.

As an aside, when doing verbose debugging, the log still shows the first virtual
server when doing the upload. It's not as an important issue as the request
body, but certainly something to be aware of when you have the time to look.
Naturally, I've changed domain names below! ;)

www.upload.vs 192.168.1.2 - - [30/Nov/2018:08:01:40 -0700] "GET 
/support/file_upload.php?iss_id=917 HTTP/1.1" 200 0
www.upload.vs 192.168.1.2 - - [30/Nov/2018:08:01:47 -0700] "POST 
/support/ajax/upload.php?file=dropfile HTTP/1.1" 200 0
# wrong virt serv #
server first_vs_in_httpd.conf, client 1 (1 active), 192.168.1.2:15244 -> 
10.0.0.10:443, closed
www.upload.vs 192.168.1.2 - - [30/Nov/2018:08:01:51 -0700] "POST 
/support/file_upload.php HTTP/1.1" 200 0
www.upload.v 192.168.1.2 - - [30/Nov/2018:08:01:53 -0700] "GET 
/support/view.php?id=917 HTTP/1.1" 200 0

Thanks,
Tracey



Re: athn(4) hostap: mem leak

2018-11-30 Thread Stefan Sperling
On Fri, Nov 30, 2018 at 01:49:56PM +0100, Benjamin Baier wrote:
> Hi
> 
> There is a leak of *arg in 
> dev/usb/if_athn_usb.c:athn_usb_newauth() line 1263
> since Rev. 1.49
> Because athn_usb_do_async() memcpy's the argument anyway.
> 
> Found with llvm/scan-build.
> 
> Instead of adding free(arg) I opted to make this function
> more like the other ones which call athn_usb_do_async.
> 
> Only compile tested... looking for tests.
> 
> Greetings Ben

Looks good to me. I would appreciate someone else testing this, too.

> 
> Index: if_athn_usb.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_athn_usb.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 if_athn_usb.c
> --- if_athn_usb.c 6 Sep 2018 11:50:54 -   1.51
> +++ if_athn_usb.c 29 Nov 2018 18:33:40 -
> @@ -1231,7 +1231,7 @@ athn_usb_newauth(struct ieee80211com *ic
>   struct ifnet *ifp = &ic->ic_if;
>   struct athn_node *an = (struct athn_node *)ni;
>   int nsta;
> - struct athn_usb_newauth_cb_arg *arg;
> + struct athn_usb_newauth_cb_arg arg;
>  
>   if (ic->ic_opmode != IEEE80211_M_HOSTAP)
>   return 0;
> @@ -1254,12 +1254,9 @@ athn_usb_newauth(struct ieee80211com *ic
>* In a process context, try to add this node to the
>* firmware table and confirm the AUTH request.
>*/
> - arg = malloc(sizeof(*arg), M_DEVBUF, M_NOWAIT);
> - if (arg == NULL)
> - return ENOMEM;
> - arg->ni = ieee80211_ref_node(ni);
> - arg->seq = seq;
> - athn_usb_do_async(usc, athn_usb_newauth_cb, arg, sizeof(*arg));
> + arg.ni = ieee80211_ref_node(ni);
> + arg.seq = seq;
> + athn_usb_do_async(usc, athn_usb_newauth_cb, &arg, sizeof(arg));
>   return EBUSY;
>  #else
>   return 0;
> 



Re: fuse_opt.3 patch

2018-11-30 Thread Edgar Pettijohn
Here is a diff to remove the dubious example and fix one gramatical issue. 

Index: fuse_opt.3
===
RCS file: /cvs/src/lib/libfuse/fuse_opt.3,v
retrieving revision 1.2
diff -u -p -u -r1.2 fuse_opt.3
--- fuse_opt.3  8 Jul 2018 06:17:10 -   1.2
+++ fuse_opt.3  30 Nov 2018 13:05:22 -
@@ -224,7 +224,7 @@ automatically depending on the format of
 If
 .Fa proc
 is not NULL, then this is called for any argument that matches a template
-with that has
+that has
 .Fa val
 = FUSE_OPT_KEY.
 .Fa opts
@@ -272,47 +272,6 @@ return 0 on success, -1 on error.
 .Pp
 .Fn fuse_opt_match
 returns 1 on match, 0 if no match.
-.Sh EXAMPLES
-.Bd -literal -offset indent
-struct foo_config {
-   char *foor;
-   int bar;
-};
-
-#define FOO_OPT(t, m) {t, offsetof(struct foo_config, m), 1}
-
-struct fuse_opt opts[] {
-   FUSE_OPT_KEY("--foo ",  KEY_FOO),
-   FOO_OPT("-b",   bar),
-   FOO_OPT("gid=", set_gid), /* set to 1 if present */
-   FOO_OPT("gid=%u",   gid),   /* set to parsed value of %u */
-   FUSE_OPT_END
-}
-
-int
-foo_process_proc(void *data, int key, char *val, struct fuse_args *args)
-{
-   foo_cofig *conf = data;
-
-   switch(key)
-   {
-   case KEY_FOO:
-   /* Do something... */
-   conf.foo = val;
-   return (0); /* discard */
-   }
-
-   /* didn't process so keep the option */
-   return (1);
-}
-
-int
-main(int argc, char *argv[])
-{
-   struct fuse_args args = FUSE_ARGS_INIT(argc, argv);
-
-   if (fuse_opt_parse(opts, config, foo_process_proc) != 0) {
-   ...
 .Ed
 .Sh ERRORS
 .Fn fuse_opt_add_arg ,



athn(4) hostap: mem leak

2018-11-30 Thread Benjamin Baier
Hi

There is a leak of *arg in 
dev/usb/if_athn_usb.c:athn_usb_newauth() line 1263
since Rev. 1.49
Because athn_usb_do_async() memcpy's the argument anyway.

Found with llvm/scan-build.

Instead of adding free(arg) I opted to make this function
more like the other ones which call athn_usb_do_async.

Only compile tested... looking for tests.

Greetings Ben

Index: if_athn_usb.c
===
RCS file: /cvs/src/sys/dev/usb/if_athn_usb.c,v
retrieving revision 1.51
diff -u -p -r1.51 if_athn_usb.c
--- if_athn_usb.c   6 Sep 2018 11:50:54 -   1.51
+++ if_athn_usb.c   29 Nov 2018 18:33:40 -
@@ -1231,7 +1231,7 @@ athn_usb_newauth(struct ieee80211com *ic
struct ifnet *ifp = &ic->ic_if;
struct athn_node *an = (struct athn_node *)ni;
int nsta;
-   struct athn_usb_newauth_cb_arg *arg;
+   struct athn_usb_newauth_cb_arg arg;
 
if (ic->ic_opmode != IEEE80211_M_HOSTAP)
return 0;
@@ -1254,12 +1254,9 @@ athn_usb_newauth(struct ieee80211com *ic
 * In a process context, try to add this node to the
 * firmware table and confirm the AUTH request.
 */
-   arg = malloc(sizeof(*arg), M_DEVBUF, M_NOWAIT);
-   if (arg == NULL)
-   return ENOMEM;
-   arg->ni = ieee80211_ref_node(ni);
-   arg->seq = seq;
-   athn_usb_do_async(usc, athn_usb_newauth_cb, arg, sizeof(*arg));
+   arg.ni = ieee80211_ref_node(ni);
+   arg.seq = seq;
+   athn_usb_do_async(usc, athn_usb_newauth_cb, &arg, sizeof(arg));
return EBUSY;
 #else
return 0;



Re: httpd server configuration evaluation bug

2018-11-30 Thread Florian Obser
On Fri, Oct 26, 2018 at 03:08:11PM -0600, Tracey Emery wrote:
> On Mon, Jul 30, 2018 at 10:24:03AM -0600, Base Pr1me wrote:
> > Sorry, this time with the correct diff.
> > 
> > On 7/25/18 4:15 PM, Base Pr1me wrote:
> > > Hi,
> > > 
> > > I discovered that the wrong server configuration is evaluated in the
> > > server_read_http function. Only the first server in httpd.conf is 
> > > checked. For
> > > example, I have five servers setup in httpd.conf and the third server is 
> > > the
> > > only one with connection { max request body } set, because I desire 
> > > it to
> > > accept larger uploads than the other servers. When the upload is 
> > > initiated,
> > > server one dictates the max request body size, globally.
> > > 
> > > The attached diff moves the queue loop out of the server_response 
> > > function in to
> > > its own function, as to not duplicate code.
> > > 
> > > I don't know if this is the only place the wrong information is 
> > > evaluated. Also,
> > > I'm not sure this is the best method to fix the problem, but it should 
> > > point the
> > > powers that be in the right direction.
> > > 
> > > Thanks,
> > > 
> > > Tracey
> > > 
> > 
> 
> Hello,
> 
> I reworked the last sent diff. I missed fixing up the hostname. This was 
> causing
> an incorrect 301 on urls not containing an ending slash. I also moved the
> srv_conf assignment into the new function.
> 
> Again, this is to use the correct server config information from the queue for
> server_read_http and remove code duplication.
> 
> If anyone is willing to look at this and make suggestions, that'd be great. If
> not, that'd be great too! LOL. Have a great weekend.
> 
> Thanks,
> Tracey

(moving back from bugs@)

Sorry for slacking of on this for so long and thanks for prodding
patiently :)

The issue is that in server_read_http we haven't found the correct
virtual server & location yet.
clt->clt_srv_conf contains the server_config struct for the listening IP.

But there is no need to check maxrequestbody just yet, we can do it in
server_response() where we do know the virtual host.

Tracey, can you please test this, it should solve your problem.

OK?

diff --git server_http.c server_http.c
index e05cec56dfc..52698a66b2e 100644
--- server_http.c
+++ server_http.c
@@ -198,7 +198,6 @@ void
 server_read_http(struct bufferevent *bev, void *arg)
 {
struct client   *clt = arg;
-   struct server_config*srv_conf = clt->clt_srv_conf;
struct http_descriptor  *desc = clt->clt_descreq;
struct evbuffer *src = EVBUFFER_INPUT(bev);
char*line = NULL, *key, *value;
@@ -357,11 +356,6 @@ server_read_http(struct bufferevent *bev, void *arg)
server_abort_http(clt, 500, errstr);
goto abort;
}
-   if ((size_t)clt->clt_toread >
-   srv_conf->maxrequestbody) {
-   server_abort_http(clt, 413, NULL);
-   goto abort;
-   }
}
 
if (strcasecmp("Transfer-Encoding", key) == 0 &&
@@ -1334,6 +1328,12 @@ server_response(struct httpd *httpd, struct client *clt)
srv_conf = server_getlocation(clt, desc->http_path);
}
 
+   if (clt->clt_toread > 0 && (size_t)clt->clt_toread >
+   srv_conf->maxrequestbody) {
+   server_abort_http(clt, 413, NULL);
+   return (-1);
+   }
+
if (srv_conf->flags & SRVFLAG_BLOCK) {
server_abort_http(clt, srv_conf->return_code,
srv_conf->return_uri);



-- 
I'm not entirely sure you are real.