bug: 9pfile(2) crash on Tread directory without Topen

bug: 9pfile(2) crash on Tread directory without Topen

[Nick Owens]

when using 9pfile, some hooks are added to lib9p Srv functions to make
file trees work. when reading a directory, some extra state is stuffed
into the fid upon Topen and used during Tread. however, it is possible
to Tread without Topen.

reproducer:

ramfs -S crash

aux/9pcon /srv/crash <<EOF
Tversion 8192 9P2000
Tattach 0 -1 $user ''
Tcreate 0 dir 020000000777 0
Tattach 5 -1 $user ''
Twalk 5 6 dir
Tread 6 0 512
EOF

crash:

term% rc crash.rc
	-> Tversion tag 65535 msize 8192 version '9P2000'
	<- Rversion tag 65535 msize 8192 version '9P2000'
	-> Tattach tag 1 fid 0 afid -1 uname glenda aname 
	<- Rattach tag 1 qid (0000000000000000 0 d)
	-> Tcreate tag 2 fid 0 name dir perm 020000000777 mode 0
	<- Rcreate tag 2 qid (0000000000000000 0 d) iounit 0 
	-> Tattach tag 3 fid 5 afid -1 uname glenda aname 
	<- Rattach tag 3 qid (0000000000000000 0 d)
	-> Twalk tag 4 fid 5 newfid 6 nwname 1 0:dir 
	<- Rwalk tag 4 nwqid 1 0:(0000000000000000 0 d) 
	-> Tread tag 5 fid 6 offset 0 count 512
ramfs 44816: suicide: sys: trap: fault read addr=0x0 pc=0x20d17f
term% acid 44816
/proc/44816/text:amd64 plan 9 executable
/sys/lib/acid/port
/sys/lib/acid/amd64
acid: lstk()
readdirfile(n=0x200,r=0x0,o=0x0,buf=0x40c258)+0x1e /sys/src/lib9p/file.c:403
	fl=0x6
	m=0x40c258
sread(srv=0x400128,r=0x40c8f8)+0x17c /sys/src/lib9p/srv.c:516
srvwork()+0x1f6 /sys/src/lib9p/srv.c:752
	srv=0x400128
	r=0x40c8f8
srv(srv=0x400128)+0x13c /sys/src/lib9p/srv.c:832
postproc()+0x34 /sys/src/lib9p/post.c:16
	s=0x400128
rforker(fn=0x20b627,arg=0x400128,flag=0x9)+0x30 /sys/src/lib9p/rfork.c:17
postsrv(s=0x400128,name=0x7fffffffefb2)+0x148 /sys/src/lib9p/post.c:44
	fd=0x500000004
	buf=0x6172632f7672732f
	cfd=0x400000006
_postmountsrv(s=0x400128,name=0x7fffffffefb2,mtpt=0x0,flag=0x4)+0x18 /sys/src/lib9p/post.c:58
postmountsrv(name=0x7fffffffefb2,mtpt=0x0,flag=0x4)+0x32 /sys/src/lib9p/rfork.c:32
main(argc=0x0,argv=0x7fffffffef98)+0x112 /sys/src/cmd/ramfs.c:514
	srvname=0x7fffffffefb2
	mtpt=0x0
	stdio=0x400000000
	mountflags=0x4
	_argc=0x53
	_args=0x405e40
_main+0x40 /sys/src/libc/amd64/main9.s:15

a tentative patch:

diff -r e512acbc403e sys/src/lib9p/srv.c
--- a/sys/src/lib9p/srv.c	Mon Mar 09 14:24:02 2020 -0700
+++ b/sys/src/lib9p/srv.c	Sun Apr 26 18:22:15 2020 -0700
@@ -513,6 +513,10 @@
 		return;
 	}
 	if((r->fid->qid.type&QTDIR) && r->fid->file){
+		if(r->fid->rdir == nil){
+			respond(r, "directory not open");
+			return;
+		}
 		r->ofcall.count = readdirfile(r->fid->rdir, r->rbuf, r->ifcall.count, r->ifcall.offset);
 		respond(r, nil);
 		return;

Re: [9front] bug: 9pfile(2) crash on Tread directory without Topen

[cinap_lenrek]

its more fundamental than that i think. both sread()
and swrite() have to check if the file has actually
been opend. fid->omode will be -1 when the file is
not open.

something like this: (untested)

http://okturing.com/src/8392/body

--
cinap

Re: [9front] bug: 9pfile(2) crash on Tread directory without Topen

[Nick Owens]

On Mon, Apr 27, 2020 at 05:32:51AM +0200, cinap_lenrek@felloff.net wrote:
> its more fundamental than that i think. both sread()
> and swrite() have to check if the file has actually
> been opend. fid->omode will be -1 when the file is
> not open.
> 
> something like this: (untested)
> 
> http://okturing.com/src/8392/body

tested, lgtm.

the reproducer now returns Ebotch and no crash.

> 
> --
> cinap

Re: [9front] bug: 9pfile(2) crash on Tread directory without Topen

[cinap_lenrek]

pushed a fix.

--
cinap