Re: uvm_fault: ip_ctloutput
This thwarts the reproducer. Again, I don't know if the invariants are getting violated somewhere else and the patch below is simply papering over the symptoms. Please include with the fix: Reported-by: syzbot+02168317bd0156c13...@syzkaller.appspotmail.com diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c index c963f7c5014..a430d2155cb 100644 --- a/sys/netinet/ip_output.c +++ b/sys/netinet/ip_output.c @@ -860,7 +860,7 @@ ip_ctloutput(int op, struct socket *so, int level, int optname, int error = 0; u_int rtid = 0; - if (level != IPPROTO_IP) { + if (inp == NULL || level != IPPROTO_IP) { error = EINVAL; } else switch (op) { case PRCO_SETOPT: On Fri, Nov 30, 2018 at 8:08 PM Greg Steuck wrote: > 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 12470
Re: uvm_fault: ip_ctloutput
On Sat, Dec 01, 2018 at 06:44:31PM -0800, Greg Steuck wrote: > This thwarts the reproducer. Again, I don't know if the invariants are > getting violated somewhere else and the patch below is simply papering over > the symptoms. I would like to better understand how we get so far with a socket where so_pcb is not initiallized. This and also the other bug are baisically the same. The stack assumes that after a successful socket() operation both socket and pcb exist and are a connected. Since this seems to not be the case it is important to catch those errors further up in uipc_socket.c before passing down into protocol specific functions. > Please include with the fix: > Reported-by: syzbot+02168317bd0156c13...@syzkaller.appspotmail.com > > diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c > index c963f7c5014..a430d2155cb 100644 > --- a/sys/netinet/ip_output.c > +++ b/sys/netinet/ip_output.c > @@ -860,7 +860,7 @@ ip_ctloutput(int op, struct socket *so, int level, int > optname, > int error = 0; > u_int rtid = 0; > > - if (level != IPPROTO_IP) { > + if (inp == NULL || level != IPPROTO_IP) { > error = EINVAL; > } else switch (op) { > case PRCO_SETOPT: > > On Fri, Nov 30, 2018 at 8:08 PM Greg Steuck wrote: > > > 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
Re: uvm_fault: ip_ctloutput
On Sun, Dec 02, 2018 at 09:29:23AM +0100, Claudio Jeker wrote: > On Sat, Dec 01, 2018 at 06:44:31PM -0800, Greg Steuck wrote: > > This thwarts the reproducer. Again, I don't know if the invariants are > > getting violated somewhere else and the patch below is simply papering over > > the symptoms. > > I would like to better understand how we get so far with a socket where > so_pcb is not initiallized. This and also the other bug are baisically the > same. The stack assumes that after a successful socket() operation both > socket and pcb exist and are a connected. Since this seems to not be > the case it is important to catch those errors further up in uipc_socket.c > before passing down into protocol specific functions. > So the issue is the double connect() call on the SOCk_RAW socket. The second connect is calling PRU_DISCONNECT which in the end does a FALLTHROUGH into PRU_ABORT which removes the inp by calling in_pcbdetach(). I think the proper fix is to not have this FALLTHROUGH and just call soisdisconnected(). Maybe inp->inp_faddr should also be reset to 0. This will fix also other double connect() SOCk_RAW crashes you spotted. -- :wq Claudio Index: raw_ip.c === RCS file: /cvs/src/sys/netinet/raw_ip.c,v retrieving revision 1.115 diff -u -p -r1.115 raw_ip.c --- raw_ip.c10 Nov 2018 18:40:34 - 1.115 +++ raw_ip.c2 Dec 2018 09:52:58 - @@ -385,7 +385,8 @@ rip_usrreq(struct socket *so, int req, s error = ENOTCONN; break; } - /* FALLTHROUGH */ + soisdisconnected(so); + break; case PRU_ABORT: soisdisconnected(so); if (inp == NULL)
Re: uvm_fault: ip_ctloutput
Awesome, thanks Claudio. As you predicted this nailed these 4 repros with a single patch :) Reported-by: syzbot+2cd350dfe5c96f646...@syzkaller.appspotmail.com Reported-by: syzbot+139ac2d7d3d601623...@syzkaller.appspotmail.com Reported-by: syzbot+02168317bd0156c13...@syzkaller.appspotmail.com Reported-by: syzbot+de8d2459ecf4cdc57...@syzkaller.appspotmail.com On Sun, Dec 2, 2018 at 2:15 AM Claudio Jeker wrote: > On Sun, Dec 02, 2018 at 09:29:23AM +0100, Claudio Jeker wrote: > > On Sat, Dec 01, 2018 at 06:44:31PM -0800, Greg Steuck wrote: > > > This thwarts the reproducer. Again, I don't know if the invariants are > > > getting violated somewhere else and the patch below is simply papering > over > > > the symptoms. > > > > I would like to better understand how we get so far with a socket where > > so_pcb is not initiallized. This and also the other bug are baisically > the > > same. The stack assumes that after a successful socket() operation both > > socket and pcb exist and are a connected. Since this seems to not be > > the case it is important to catch those errors further up in > uipc_socket.c > > before passing down into protocol specific functions. > > > > So the issue is the double connect() call on the SOCk_RAW socket. > The second connect is calling PRU_DISCONNECT which in the end does a > FALLTHROUGH into PRU_ABORT which removes the inp by calling > in_pcbdetach(). > > I think the proper fix is to not have this FALLTHROUGH and just call > soisdisconnected(). Maybe inp->inp_faddr should also be reset to 0. > > This will fix also other double connect() SOCk_RAW crashes you spotted. > -- > :wq Claudio > > Index: raw_ip.c > === > RCS file: /cvs/src/sys/netinet/raw_ip.c,v > retrieving revision 1.115 > diff -u -p -r1.115 raw_ip.c > --- raw_ip.c10 Nov 2018 18:40:34 - 1.115 > +++ raw_ip.c2 Dec 2018 09:52:58 - > @@ -385,7 +385,8 @@ rip_usrreq(struct socket *so, int req, s > error = ENOTCONN; > break; > } > - /* FALLTHROUGH */ > + soisdisconnected(so); > + break; > case PRU_ABORT: > soisdisconnected(so); > if (inp == NULL) > -- nest.cx is Gmail hosted, use PGP for anything private. Key: http://goo.gl/6dMsr Fingerprint: 5E2B 2D0E 1E03 2046 BEC3 4D50 0B15 42BD 8DF5 A1B0
Re: uvm_fault: ip_ctloutput
On Sun, Dec 02, 2018 at 11:15:03AM +0100, Claudio Jeker wrote: > On Sun, Dec 02, 2018 at 09:29:23AM +0100, Claudio Jeker wrote: > > On Sat, Dec 01, 2018 at 06:44:31PM -0800, Greg Steuck wrote: > > > This thwarts the reproducer. Again, I don't know if the invariants are > > > getting violated somewhere else and the patch below is simply papering > > > over > > > the symptoms. > > > > I would like to better understand how we get so far with a socket where > > so_pcb is not initiallized. This and also the other bug are baisically the > > same. The stack assumes that after a successful socket() operation both > > socket and pcb exist and are a connected. Since this seems to not be > > the case it is important to catch those errors further up in uipc_socket.c > > before passing down into protocol specific functions. > > > > So the issue is the double connect() call on the SOCk_RAW socket. > The second connect is calling PRU_DISCONNECT which in the end does a > FALLTHROUGH into PRU_ABORT which removes the inp by calling > in_pcbdetach(). > > I think the proper fix is to not have this FALLTHROUGH and just call > soisdisconnected(). Maybe inp->inp_faddr should also be reset to 0. > > This will fix also other double connect() SOCk_RAW crashes you spotted. The version I will commit will also reset inp->inp_faddr since this is what other protos are doing and it is more correct anyway. -- :wq Claudio Index: raw_ip.c === RCS file: /cvs/src/sys/netinet/raw_ip.c,v retrieving revision 1.115 diff -u -p -r1.115 raw_ip.c --- raw_ip.c10 Nov 2018 18:40:34 - 1.115 +++ raw_ip.c3 Dec 2018 08:13:24 - @@ -385,7 +385,9 @@ rip_usrreq(struct socket *so, int req, s error = ENOTCONN; break; } - /* FALLTHROUGH */ + soisdisconnected(so); + inp->inp_faddr.s_addr = INADDR_ANY; + break; case PRU_ABORT: soisdisconnected(so); if (inp == NULL)