Pekka Enberg wrote: > > I'll put them on my todo and in the meanwhile, you can find the latest > patches here: > http://www.kernel.org/pub/linux/kernel/people/penberg/patches/revoke/ > > Thanks for taking the time to review the patch! + err = close_files(this); + + put_task_struct(this->owner); + if (err) + break; + } + if (err) + restore_files(&to_cleanup[i], nr_fds-i); I think, the error path is wrong as it tries to restore "this" which means the now invalid filp (close always closes, even in case of errors) is put back into the fd-table; and, the task struct is put twice. I think, you should ignore errors on close. (But I guess, this part of revoke gets rewritten anyway to match BSD behaviour.) I wonder, if revoke should really abort when there's an error from one fd or better continue and try its best. restore_files: + spin_lock(&files->file_lock); + fdt = files_fdtable(files); + rcu_assign_pointer(fdt->fd[this->fd], this->file); + FD_SET(this->fd, fdt->close_on_exec); + spin_unlock(&files->file_lock); + put_files_struct(files); + } + + put_task_struct(this->owner); This sets close_on_exec unconditionally, even if it wasn't set before. Hm..., if a cloned thread is able to exec, it seems a little bit dangerous to restore the fd-table with filps that were valid some time ago - the fd-table may have changed in the meantime... But maybe I simply missed something... Ciao, ET. - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
