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 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: athn(4) hostap: mem leak
On Sat, 1 Dec 2018 15:48:13 -0200 Martin Pieuchot wrote: > On 30/11/18(Fri) 13:49, 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. > > You should also remove the free(arg...) in athn_usb_newauth_cb(). Indeed, new patch attached. 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 2 Dec 2018 09:09:29 - @@ -1202,8 +1202,6 @@ athn_usb_newauth_cb(struct athn_usb_soft struct athn_node *an = (struct athn_node *)ni; int s, error = 0; - free(arg, M_DEVBUF, sizeof(*arg)); - if (ic->ic_state != IEEE80211_S_RUN) return; @@ -1231,7 +1229,7 @@ athn_usb_newauth(struct ieee80211com *ic struct ifnet *ifp = >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 +1252,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, , sizeof(arg)); return EBUSY; #else return 0;
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
> > 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 20x100090
