[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Dazuko-devel] Bug in mask_proc (repost)
From: |
John Ogness |
Subject: |
Re: [Dazuko-devel] Bug in mask_proc (repost) |
Date: |
Wed, 25 Nov 2009 22:40:25 +0100 |
User-agent: |
Gnus/5.11 (Gnus v5.11) Emacs/22.2 (gnu/linux) |
On 2009-11-24, Lino Sanfilippo <address@hidden> wrote:
>> This problem can occur every time after the daemon that ignored
>> itself terminates (unexpectedly or not) before it is able to remove
>> its proc structure from the list.
>
> Ok, forget about that. Of course that cant happen, since a signal
> (like SIGTERM) sent to the process wont keep the kernel code from
> cleaning up before the process is terminated. So it should always be
> able to clean up its own list entry :P
I actually thought a lot about this when I first implemented it. And I
just finished investigating it again. It is assumed that no preemption
occurs in the calling chain between event.c:dentry_open() and
file.c:dazukofs_open(). Right now that assumption is correct, but it
could change at some point in the future.
If preemption did occur, it would be possible for
dazukofs_remove_group() to be called, which could result in
check_access_precheck() aborting without removing the process from the
list (if group_count was now 0). This would then lead to invalid
memory in the list, because open_file() also would not remove it.
Probably a good (and efficient) fix would be to have check_recursion()
set a "removed" flag in the process structure. That way, open_file()
could very easily see if it has been removed or not. In my opinion,
that is much better than relying on the return value of dentry_open().
Rather than:
ec->file = dentry_open(dget(evt->dentry), mntget(evt->mnt),
O_RDONLY | O_LARGEFILE, current_cred());
if (IS_ERR(ec->file)) {
check_recursion(); /* remove myself from proc_list */
ret = PTR_ERR(ec->file);
goto error_out2;
}
the code could look like this:
ec->file = dentry_open(dget(evt->dentry), mntget(evt->mnt),
O_RDONLY | O_LARGEFILE, current_cred());
if (!proc.removed)
check_recursion(); /* remove myself from proc_list */
if (IS_ERR(ec->file)) {
ret = PTR_ERR(ec->file);
goto error_out2;
}
John Ogness
--
Dazuko Maintainer