>>>>> " " == Ion Badulescu <[email protected]> writes:
> Ugh, that's ugly -- and potentially a DoS issue as well. Since
> you don't need any privileges to issue the 'kill -9' for your
> own processes, it becomes fairly easy to write some code that
> exploits this and causes kernel crashes across the board...
Then don't set the "intr" flag.
> Not that the current situation is any better, of course.
> Perhaps making that wait an uninterruptable wait would be
> enough to take care of the problem?
People expect the "intr" flag to work if they set it.
Cheers,
Trond
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Fri, Sep 26, 2003 at 03:29:40PM -0700, Trond Myklebust wrote:
> > Not that the current situation is any better, of course.
>
> > Perhaps making that wait an uninterruptable wait would be
> > enough to take care of the problem?
>
> People expect the "intr" flag to work if they set it.
In this case, not removing the sillyrenamed file instead
of oopsing may be the preferred action.
Olaf
--
Olaf Kirch | Anyone who has had to work with X.509 has probably
[email protected] | experienced what can best be described as
---------------+ ISO water torture. -- Peter Gutmann
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On 2003.09.27 17:55, Olaf Kirch wrote:
> In this case, not removing the sillyrenamed file instead
> of oopsing may be the preferred action.
Would it be possibly to combine both ideas, i.e. have the last process
wait, but if the user attempt to kill that process and it will be
allowed because intr is set the sillyrenamed file is not removed.
Surely it is reasonable for people who kill -9 things to expect to do
some tidying up, but not to expect a kernel panic.
Steve.
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Fri, 26 Sep 2003, Trond Myklebust wrote:
> > Perhaps making that wait an uninterruptable wait would be
> > enough to take care of the problem?
>
> People expect the "intr" flag to work if they set it.
People also expect the supported "intr" flag not to cause security
problems and would gladly take an unkillable process over a kernel oops...
Ion
--
It is better to keep your mouth shut and be thought a fool,
than to open it and remove all doubt.
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
>>>>> " " == Ion Badulescu <[email protected]> writes:
>> People expect the "intr" flag to work if they set it.
> People also expect the supported "intr" flag not to cause
> security problems and would gladly take an unkillable process
> over a kernel oops...
You'd have to be extremely unlucky to kill the process and hit the
window for the Oops. I defy you to come up with an exploit for it.
That said, I agree that a full fix would be preferable. I'm working on
other projects right now, that's why I'm being slow about this issue
(plus the fact that it's not exactly easy to reproduce). I'll get onto
it soon...
Cheers,
Trond
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Mon, Sep 29, 2003 at 10:22:40AM -0700, Trond Myklebust wrote:
> You'd have to be extremely unlucky to kill the process and hit the
> window for the Oops. I defy you to come up with an exploit for it.
>
> That said, I agree that a full fix would be preferable. I'm working on
> other projects right now, that's why I'm being slow about this issue
> (plus the fact that it's not exactly easy to reproduce). I'll get onto
> it soon...
I've had no luck reproducing it either in a controlled environment.
However the bug is common enough to crash a few of our build machines
over a weekend...
I took your patch, Trond, and modified it a little to not crash if
the user ctrl-c's the unlink. Still not pretty - maybe it should
zap the cached attributes:
@@ -212,7 +222,15 @@
data->count++;
nfs_copy_dname(dentry, data);
dentry->d_flags &= ~DCACHE_NFSFS_RENAMED;
- if (data->task.tk_rpcwait == &nfs_delete_queue)
+ if (data->task.tk_rpcwait == &nfs_delete_queue) {
+ struct rpc_clnt *clnt = data->task.tk_client;
rpc_wake_up_task(&data->task);
+ nfs_wait_event(clnt, data->waitq, data->completed == 1);
+ /* This is safe as we hold the BKL */
+ if (!data->completed) {
+ dput(data->dir);
+ data->dir = NULL;
+ }
+ }
nfs_put_unlinkdata(data);
}
Olaf
--
Olaf Kirch | Anyone who has had to work with X.509 has probably
[email protected] | experienced what can best be described as
---------------+ ISO water torture. -- Peter Gutmann
>>>>> " " == Olaf Kirch <[email protected]> writes:
> On Mon, Sep 29, 2003 at 10:22:40AM -0700, Trond Myklebust
>> That said, I agree that a full fix would be preferable. I'm
>> working on other projects right now, that's why I'm being slow
>> about this issue (plus the fact that it's not exactly easy to
>> reproduce). I'll get onto it soon...
> I've had no luck reproducing it either in a controlled
> environment. However the bug is common enough to crash a few
> of our build machines over a weekend...
> I took your patch, Trond, and modified it a little to not crash
> if the user ctrl-c's the unlink. Still not pretty - maybe it
> should zap the cached attributes:
Alternatively, we should perhaps make fsync_super() sync out all dirty
inodes and readaheads, then wait on all pending RPC ops. That would be
cleaner, I think...
Cheers,
Trond
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs