xfnw/plan9fox
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

kernel: attachimage / exec error handling

Browse source 
attachimage()'s approach to handling newseg() error is flawed:

a) the the image is on the hash table, but ref is still 0, and
there is no segment/pages attached to it so nobody is going to
reclaim / putimage() it -> leak

b) calling pexit() would deadlock us because exec has acquired
up->seglock when calling attachimage(), so this would just deadlock.

the fix does the following:

attachimage() will putimage() and nexterror() if newseg() fails
instead of pexit(). this is less surprising.

exec now keeps the condition variable commit which is set once
we are commited / reached the point of no return and check this
variable in the highest waserror() handler and pexit() us there.

this way we have released up all the locks and pexit() will
cleanup.

note: this bug shouldnt us hit in with the current newseg()
implementation as it uses smalloc() which would wait to
satisfy the allocation instead of erroring.
This commit is contained in:
cinap_lenrek 2012-10-14 19:48:46 +02:00
parent fef6ff96ad
commit 2f732e9a85
2 changed files with 13 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
sys/src/9/port/segment.c
View file

@ -285,14 +285,14 @@ found:
unlock(&imagealloc);
if(i->s == 0) {
/* Disaster after commit in exec */
i->ref++;
if(waserror()) {
unlock(i);
pexit(Enovmem, 1);
putimage(i);
nexterror();
}
i->s = newseg(type, base, len);
i->s->image = i;
i->ref++;
poperror();
}
else

13
sys/src/9/port/sysproc.c
View file

@ -228,7 +228,7 @@ sysexec(ulong *arg)
char *a, *charp, *args, *file, *file0;
char *progarg[sizeof(Exec)/2+1], *elem, progelem[64];
ulong ssize, spage, nargs, nbytes, n, bssend;
int indir;
int indir, commit;
Exec exec;
char line[sizeof(Exec)];
Fgrp *f;
@ -236,6 +236,7 @@ sysexec(ulong *arg)
ulong magic, text, entry, data, bss;
Tos *tos;
commit = 0;
indir = 0;
elem = nil;
validaddr(arg[0], 1, 0);
@ -243,6 +244,9 @@ sysexec(ulong *arg)
if(waserror()){
free(file0);
free(elem);
/* Disaster after commit */
if(commit)
pexit(up->errstr, 1);
nexterror();
}
file = file0;
@ -407,6 +411,9 @@ sysexec(ulong *arg)
}
up->nargs = n;
commit = 1;
USED(commit);
/*
* Committed.
* Free old memory.
@ -466,7 +473,6 @@ sysexec(ulong *arg)
up->seg[SSEG] = s;
qunlock(&up->seglock);
poperror(); /* seglock */
poperror(); /* elem */
/*
* '/' processes are higher priority (hack to make /ip more responsive).
@ -474,8 +480,9 @@ sysexec(ulong *arg)
if(devtab[tc->type]->dc == L'/')
up->basepri = PriRoot;
up->priority = up->basepri;
poperror();
cclose(tc);
poperror(); /* tc */
poperror(); /* elem */
qlock(&up->debug);
up->nnote = 0;