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.