Re: [Dazuko-devel] Bug in mask_proc (repost)

[John Ogness]

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