On Fri, Sep 26, 2025 at 07:48:01AM +0100, Al Viro wrote:
> On Fri, Sep 26, 2025 at 12:00:08AM +0530, Naresh Kamboju wrote:
>
> [snip]
>
> With 59bfb6681680 "listmount: don't call path_put() under namespace semaphore"
> we get this:
>
> static void __free_klistmount_free(const struct klistmount *kls)
> {
> path_put(&kls->root);
> kvfree(kls->kmnt_ids);
> mnt_ns_release(kls->ns);
> }
>
> ...
>
> SYSCALL_DEFINE4(listmount, const struct mnt_id_req __user *, req,
> u64 __user *, mnt_ids, size_t, nr_mnt_ids, unsigned int, flags)
> {
> struct klistmount kls __free(klistmount_free) = {};
> const size_t maxcount = 1000000;
> struct mnt_id_req kreq;
> ssize_t ret;
>
> if (flags & ~LISTMOUNT_REVERSE)
> return -EINVAL;
>
> which will oops if it takes that failure exit - if you are initializing
> something with any kind of cleanup on it, you'd better make sure
> the cleanup will survive being called for the initial value...
>
> Christian, that's your branch and I don't want to play with rebasing
> it - had it been mine, the fix would be folded into commit in question,
> with the rest of the branch cherry-picked on top of fixed commit,
> but everyone got their own preferences in how to do such stuff.
>
> Minimal fix would be to make mnt_ns_release(NULL) a no-op.
>
> BTW, I suspect that one of the sources of confusion had been the fact that
> __free(mnt_ns_release) *does* treat NULL as no-op; in statmount(2) you
> are using that and get away with NULL as initializer. In listmount(2)),
> OTOH, you are dealing with the function call - same identifier, different
> behaviour...
Ah, fuck me. Thanks for spotting that! I'll take care of it.
