* hjfs stack overflow @ 2018-08-16 2:44 Nick Owens 2018-08-16 12:03 ` [9front] " Julius Schmidt 0 siblings, 1 reply; 3+ messages in thread From: Nick Owens @ 2018-08-16 2:44 UTC (permalink / raw) To: 9front there is a stack overflow in hjfs, trivially reproducible by executing: # set up a new temporary disk dd -if /dev/zero -of /tmp/hjfs.tmp -oseek 52428800 -count 1 hjfs -n hjfs.tmp -f /tmp/hjfs.tmp -S -r -m 16 echo newuser $user > /srv/hjfs.tmp.cmd mount -cC /srv/hjfs.tmp /n/hjfs.tmp cd /n/hjfs.tmp # crash hjfs mkdir -p `{seq 1000 | tr '\xa' /} the function willmodify is recursive, and appears to recurse for each element in the directory path. possible solutions i see are make it iterative, or have it call needstack() from libthread and return an error when out of stack. below is a stack trace with the majority of the recursive calls clipped out. /proc/143864/text:amd64 plan 9 executable /sys/lib/acid/port /sys/lib/acid/amd64 $home/lib/acid acid: abort()+0x0 /sys/src/libc/9sys/abort.c:6 needstack(n=0x80)+0xa5 /sys/src/libthread/sched.c:95 x=0x217c2afefefefe _sched()+0x27 /sys/src/libthread/sched.c:109 p=0x1424da0 t=0x15b6bd8 _threadrendezvous(tag=0x15b6df0,val=0x0)+0x132 /sys/src/libthread/rendez.c:56 l=0x40dc68 t=0x15b6bd8 ret=0x1424da0 alt(alts=0x15b6e40)+0x1ed /sys/src/libthread/channel.c:172 t=0x15b6bd8 s=0x0 n=0x0 a=0x0 c=0x0 xa=0x15b6e40 ca=0x0 waiting=0x1 allreadycl=0x100000000 r=0xfefefefefefefefe runop(c=0x418500,v=0x15b6f28,nb=0xfefefefe00000000)+0x52 /sys/src/libthread/channel.c:314 a=0x418500 recv(v=0x15b6f28)+0x28 /sys/src/libthread/channel.c:321 getbuf(d=0x1435c10,off=0x4,nodata=0xfefefefe00000000,type=0xfefefefe00000004)+0x92 /sys/src/cmd/hjfs/buf.c:268 req=0x1435c10 th=0x4184c0 b=0xfefefefefefefefe chref(r=0x12c4,stat=0x0)+0x53 /sys/src/cmd/hjfs/fs1.c:18 j=0xfefefefe000002c5 rc=0x2c5fefefefe willmodify(l=0x1628098,fs=0x15fae18,nolock=0x1)+0x96 /sys/src/cmd/hjfs/dump.c:133 p=0x217f20 d=0x1425780 i=0xfefefefefefefefe r=0x20cea0 [ ... ] willmodify(l=0x423640,fs=0x15fae18,nolock=0xffffffff00000001)+0x6f /sys/src/cmd/hjfs/dump.c:131 p=0x1628d68 d=0x15fae18 i=0x21b8e7 r=0x70020cea0 willmodify(l=0x4236e0,fs=0x15fae18,nolock=0x0)+0x6f /sys/src/cmd/hjfs/dump.c:131 p=0x1424da0 d=0x20cea0 i=0x203b88 r=0x405101 chancreat(ch=0x418020,name=0x15fbb84,perm=0x800001ff,mode=0xfefefefe00000000)+0xd9 /sys/src/cmd/hjfs/fs2.c:119 b=0x0 l=0x0 isdir=0x403f3800000001 d=0x403f38 f=0x20fba0 workerproc()+0x1dd /sys/src/cmd/hjfs/9p.c:216 ch=0x418020 req=0x1628a98 o=0x1628bc0 launcheramd64(arg=0x0,f=0x2097b7)+0x10 /sys/src/libthread/amd64.c:11 0xfefefefefefefefe ?file?:0 acid: echo kill > /proc/143864/ctl ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [9front] hjfs stack overflow 2018-08-16 2:44 hjfs stack overflow Nick Owens @ 2018-08-16 12:03 ` Julius Schmidt 2018-08-16 20:12 ` Nick Owens 0 siblings, 1 reply; 3+ messages in thread From: Julius Schmidt @ 2018-08-16 12:03 UTC (permalink / raw) To: 9front [-- Attachment #1: Type: TEXT/PLAIN, Size: 2906 bytes --] Thanks for finding this. I've included a patch that uses an explicit stack. Can somebody please test it and/or review it? I've tried mischief's example and it seems to not crash but it seems mkdir or pwd truncates the path or something. On Wed, 15 Aug 2018, Nick Owens wrote: > there is a stack overflow in hjfs, trivially reproducible by executing: > > # set up a new temporary disk > dd -if /dev/zero -of /tmp/hjfs.tmp -oseek 52428800 -count 1 > hjfs -n hjfs.tmp -f /tmp/hjfs.tmp -S -r -m 16 > echo newuser $user > /srv/hjfs.tmp.cmd > mount -cC /srv/hjfs.tmp /n/hjfs.tmp > cd /n/hjfs.tmp > > # crash hjfs > mkdir -p `{seq 1000 | tr '\xa' /} > > the function willmodify is recursive, and appears to recurse for each > element in the directory path. possible solutions i see are make it > iterative, or have it call needstack() from libthread and return an error > when out of stack. > > below is a stack trace with the majority of the recursive calls clipped out. > > /proc/143864/text:amd64 plan 9 executable > /sys/lib/acid/port > /sys/lib/acid/amd64 > $home/lib/acid > acid: abort()+0x0 /sys/src/libc/9sys/abort.c:6 > needstack(n=0x80)+0xa5 /sys/src/libthread/sched.c:95 > x=0x217c2afefefefe > _sched()+0x27 /sys/src/libthread/sched.c:109 > p=0x1424da0 > t=0x15b6bd8 > _threadrendezvous(tag=0x15b6df0,val=0x0)+0x132 > /sys/src/libthread/rendez.c:56 > l=0x40dc68 > t=0x15b6bd8 > ret=0x1424da0 > alt(alts=0x15b6e40)+0x1ed /sys/src/libthread/channel.c:172 > t=0x15b6bd8 > s=0x0 > n=0x0 > a=0x0 > c=0x0 > xa=0x15b6e40 > ca=0x0 > waiting=0x1 > allreadycl=0x100000000 > r=0xfefefefefefefefe > runop(c=0x418500,v=0x15b6f28,nb=0xfefefefe00000000)+0x52 > /sys/src/libthread/channel.c:314 > a=0x418500 > recv(v=0x15b6f28)+0x28 /sys/src/libthread/channel.c:321 > getbuf(d=0x1435c10,off=0x4,nodata=0xfefefefe00000000,type=0xfefefefe00000004)+0x92 > /sys/src/cmd/hjfs/buf.c:268 > req=0x1435c10 > th=0x4184c0 > b=0xfefefefefefefefe > chref(r=0x12c4,stat=0x0)+0x53 /sys/src/cmd/hjfs/fs1.c:18 > j=0xfefefefe000002c5 > rc=0x2c5fefefefe > willmodify(l=0x1628098,fs=0x15fae18,nolock=0x1)+0x96 > /sys/src/cmd/hjfs/dump.c:133 > p=0x217f20 > d=0x1425780 > i=0xfefefefefefefefe > r=0x20cea0 > > > [ ... ] > > > willmodify(l=0x423640,fs=0x15fae18,nolock=0xffffffff00000001)+0x6f > /sys/src/cmd/hjfs/dump.c:131 > p=0x1628d68 > d=0x15fae18 > i=0x21b8e7 > r=0x70020cea0 > willmodify(l=0x4236e0,fs=0x15fae18,nolock=0x0)+0x6f > /sys/src/cmd/hjfs/dump.c:131 > p=0x1424da0 > d=0x20cea0 > i=0x203b88 > r=0x405101 > chancreat(ch=0x418020,name=0x15fbb84,perm=0x800001ff,mode=0xfefefefe00000000)+0xd9 > /sys/src/cmd/hjfs/fs2.c:119 > b=0x0 > l=0x0 > isdir=0x403f3800000001 > d=0x403f38 > f=0x20fba0 > workerproc()+0x1dd /sys/src/cmd/hjfs/9p.c:216 > ch=0x418020 > req=0x1628a98 > o=0x1628bc0 > launcheramd64(arg=0x0,f=0x2097b7)+0x10 /sys/src/libthread/amd64.c:11 > 0xfefefefefefefefe ?file?:0 > acid: > echo kill > /proc/143864/ctl > [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: Type: TEXT/x-diff; name=hjfs.patch, Size: 2232 bytes --] diff -r f9a9f7e1c5e6 sys/src/cmd/hjfs/dump.c --- a/sys/src/cmd/hjfs/dump.c Wed Jul 11 16:05:03 2018 +0100 +++ b/sys/src/cmd/hjfs/dump.c Thu Aug 16 13:00:08 2018 +0100 @@ -112,8 +112,8 @@ return -1; } -int -willmodify(Fs *fs, Loc *l, int nolock) +static int +willmodify1(Fs *fs, Loc *l) { Buf *p; Loc *m; @@ -121,29 +121,20 @@ Dentry *d; int rc; - if((l->flags & LDUMPED) != 0) - return 1; - if(!nolock){ -again: - runlock(fs); - wlock(fs); - } - if(l->next != nil && willmodify(fs, l->next, 1) < 0) - goto err; rc = chref(fs, l->blk, 0); if(rc < 0) - goto err; + return -1; if(rc == 0){ dprint("willmodify: block %lld has refcount 0\n", l->blk); werrstr("phase error -- willmodify"); - goto err; + return -1; } if(rc == 1) goto done; p = getbuf(fs->d, l->next->blk, TDENTRY, 0); if(p == nil) - goto err; + return -1; d = getdent(l->next, p); if(d != nil) for(i = 0; i < d->size; i++){ rc = getblk(fs, l->next, p, i, &r, GBREAD); @@ -155,12 +146,12 @@ phase: werrstr("willmodify -- phase error"); putbuf(p); - goto err; + return -1; found: rc = getblk(fs, l->next, p, i, &r, GBWRITE); if(rc < 0){ putbuf(p); - goto err; + return -1; } if(rc == 0) goto phase; @@ -180,17 +171,50 @@ } done: l->flags |= LDUMPED; + return 0; +} + +int +willmodify(Fs *fs, Loc *l, int nolock) +{ + Loc **st; + int sti, rc; + + if((l->flags & LDUMPED) != 0) + return 1; + if(!nolock){ +again: + runlock(fs); + wlock(fs); + } + st = emalloc(sizeof(Loc *)); + *st = l; + sti = 0; + for(;;){ + if((st[sti]->flags & LDUMPED) != 0 || st[sti]->next == nil) + break; + st = erealloc(st, (sti + 2) * sizeof(Loc *)); + st[sti + 1] = st[sti]->next; + sti++; + } + rc = 0; + for(; sti >= 0; sti--){ + rc = willmodify1(fs, st[sti]); + if(rc < 0){ + free(st); + if(!nolock){ + wunlock(fs); + rlock(fs); + } + return -1; + } + } if(!nolock){ wunlock(fs); rlock(fs); if(chref(fs, l->blk, 0) != 1) goto again; } - return 0; -err: - if(!nolock){ - wunlock(fs); - rlock(fs); - } - return -1; + free(st); + return rc; } ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [9front] hjfs stack overflow 2018-08-16 12:03 ` [9front] " Julius Schmidt @ 2018-08-16 20:12 ` Nick Owens 0 siblings, 0 replies; 3+ messages in thread From: Nick Owens @ 2018-08-16 20:12 UTC (permalink / raw) To: 9front i tested the patch, and it is working here. hjfs no longer encounters a stack overflow while creating very deep directories. On Thu, Aug 16, 2018 at 5:03 AM, Julius Schmidt <aiju@phicode.de> wrote: > Thanks for finding this. > > I've included a patch that uses an explicit stack. > > Can somebody please test it and/or review it? > > I've tried mischief's example and it seems to not crash but it seems mkdir > or pwd truncates the path or something. > > > On Wed, 15 Aug 2018, Nick Owens wrote: > >> there is a stack overflow in hjfs, trivially reproducible by executing: >> >> # set up a new temporary disk >> dd -if /dev/zero -of /tmp/hjfs.tmp -oseek 52428800 -count 1 >> hjfs -n hjfs.tmp -f /tmp/hjfs.tmp -S -r -m 16 >> echo newuser $user > /srv/hjfs.tmp.cmd >> mount -cC /srv/hjfs.tmp /n/hjfs.tmp >> cd /n/hjfs.tmp >> >> # crash hjfs >> mkdir -p `{seq 1000 | tr '\xa' /} >> >> the function willmodify is recursive, and appears to recurse for each >> element in the directory path. possible solutions i see are make it >> iterative, or have it call needstack() from libthread and return an error >> when out of stack. >> >> below is a stack trace with the majority of the recursive calls clipped >> out. >> >> /proc/143864/text:amd64 plan 9 executable >> /sys/lib/acid/port >> /sys/lib/acid/amd64 >> $home/lib/acid >> acid: abort()+0x0 /sys/src/libc/9sys/abort.c:6 >> needstack(n=0x80)+0xa5 /sys/src/libthread/sched.c:95 >> x=0x217c2afefefefe >> _sched()+0x27 /sys/src/libthread/sched.c:109 >> p=0x1424da0 >> t=0x15b6bd8 >> _threadrendezvous(tag=0x15b6df0,val=0x0)+0x132 >> /sys/src/libthread/rendez.c:56 >> l=0x40dc68 >> t=0x15b6bd8 >> ret=0x1424da0 >> alt(alts=0x15b6e40)+0x1ed /sys/src/libthread/channel.c:172 >> t=0x15b6bd8 >> s=0x0 >> n=0x0 >> a=0x0 >> c=0x0 >> xa=0x15b6e40 >> ca=0x0 >> waiting=0x1 >> allreadycl=0x100000000 >> r=0xfefefefefefefefe >> runop(c=0x418500,v=0x15b6f28,nb=0xfefefefe00000000)+0x52 >> /sys/src/libthread/channel.c:314 >> a=0x418500 >> recv(v=0x15b6f28)+0x28 /sys/src/libthread/channel.c:321 >> >> getbuf(d=0x1435c10,off=0x4,nodata=0xfefefefe00000000,type=0xfefefefe00000004)+0x92 >> /sys/src/cmd/hjfs/buf.c:268 >> req=0x1435c10 >> th=0x4184c0 >> b=0xfefefefefefefefe >> chref(r=0x12c4,stat=0x0)+0x53 /sys/src/cmd/hjfs/fs1.c:18 >> j=0xfefefefe000002c5 >> rc=0x2c5fefefefe >> willmodify(l=0x1628098,fs=0x15fae18,nolock=0x1)+0x96 >> /sys/src/cmd/hjfs/dump.c:133 >> p=0x217f20 >> d=0x1425780 >> i=0xfefefefefefefefe >> r=0x20cea0 >> >> >> [ ... ] >> >> >> willmodify(l=0x423640,fs=0x15fae18,nolock=0xffffffff00000001)+0x6f >> /sys/src/cmd/hjfs/dump.c:131 >> p=0x1628d68 >> d=0x15fae18 >> i=0x21b8e7 >> r=0x70020cea0 >> willmodify(l=0x4236e0,fs=0x15fae18,nolock=0x0)+0x6f >> /sys/src/cmd/hjfs/dump.c:131 >> p=0x1424da0 >> d=0x20cea0 >> i=0x203b88 >> r=0x405101 >> >> chancreat(ch=0x418020,name=0x15fbb84,perm=0x800001ff,mode=0xfefefefe00000000)+0xd9 >> /sys/src/cmd/hjfs/fs2.c:119 >> b=0x0 >> l=0x0 >> isdir=0x403f3800000001 >> d=0x403f38 >> f=0x20fba0 >> workerproc()+0x1dd /sys/src/cmd/hjfs/9p.c:216 >> ch=0x418020 >> req=0x1628a98 >> o=0x1628bc0 >> launcheramd64(arg=0x0,f=0x2097b7)+0x10 /sys/src/libthread/amd64.c:11 >> 0xfefefefefefefefe ?file?:0 >> acid: >> echo kill > /proc/143864/ctl > > > from postmaster@ewsd: > The following attachment had content that we can't > prove to be harmless. To avoid possible automatic > execution, we changed the content headers. > The original header was: > > Content-Type: TEXT/x-diff; name=hjfs.patch > Content-Transfer-Encoding: BASE64 > Content-ID: <alpine.LNX.2.00.1808161403200.15422@phi> > Content-Description: > Content-Disposition: attachment; filename=hjfs.patch ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-08-16 21:24 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-16 2:44 hjfs stack overflow Nick Owens 2018-08-16 12:03 ` [9front] " Julius Schmidt 2018-08-16 20:12 ` Nick Owens
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).