From: Eric W. Biederman on 23 Mar 2010 17:10
Oleg Nesterov <oleg(a)redhat.com> writes:
> (on top of check_unshare_flags-kill-the-bogus-clone_sighand-sig-count-check.patch)
> sys_unshare(CLONE_THREAD/SIGHAND/VM) is not really implemented, and I doubt
> very much it will ever work. At least, nobody even tried since the original
> "unshare system call -v5: system call handler function" commit
> 99d1419d96d7df9cfa56bc977810be831bd5ef64 was applied more than 4 years ago.
> And the code is not consistent. unshare_thread() always fails unconditionally,
> while unshare_sighand() and unshare_vm() pretend to work if there is nothing
> to unshare.
This is setting off alarm bells in my head.
I haven't traced this all through but I like your logic a lot less, and
I think it is buggy. Why don't we need to look at sigh->count ?
The current logic is very fine grained but it does a lot of simple logical
checks and it ties those checks together if a very maintainable way.
You require that we know upfront all of the dependencies, which is things
change subtlety can be a maintenance challenge.
> Remove unshare_thread(), unshare_sighand(), unshare_vm() helpers and related
> variables and add a simple CLONE_THREAD | CLONE_SIGHAND| CLONE_VM check into
> Also, move the "CLONE_NEWNS needs CLONE_FS" check from check_unshare_flags()
> to sys_unshare(). This looks more consistent and matches the similar
> do_sysvsem check in sys_unshare().
> Note: with or without this patch "atomic_read(mm->mm_users) > 1" can give
> a false positive due to get_task_mm().
I think the number of times get_task_mm is called on not current this isn't
an interesting race.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/