Re: build_path_from_dentry_optional_prefix() may schedule from invalid context

[Pavel Shilovsky]

ÑÑ, 28 ÐÐÐ. 2019 Ð. Ð 22:02, Sergey Senozhatsky
<sergey.senozhatsky.work@xxxxxxxxx>:
>
> Hello,
>
> Looking at commit "cifs: create a helper to find a writeable handle
> by path name":
>
> ->open_file_lock scope is atomic context, while build_path_from_dentry()
> can schedule - kmalloc(GFP_KERNEL)
>
>        spin_lock(&tcon->open_file_lock);
>        list_for_each(tmp, &tcon->openFileList) {
>                cfile = list_entry(tmp, struct cifsFileInfo,
>                             tlist);
>                full_path = build_path_from_dentry(cfile->dentry);
>                if (full_path == NULL) {
>                        spin_unlock(&tcon->open_file_lock);
>                        return -ENOMEM;
>                }
>                if (strcmp(full_path, name)) {
>                        kfree(full_path);
>                        continue;
>                }
>                kfree(full_path);
>
>                cinode = CIFS_I(d_inode(cfile->dentry));
>                spin_unlock(&tcon->open_file_lock);
>                return cifs_get_writable_file(cinode, 0, ret_file);
>        }
>
>        spin_unlock(&tcon->open_file_lock);
>
> Additionally, kfree() can (and should) be done outside of
> ->open_file_lock scope.
>
>         -ss

Good catch. I think we should have another version of
build_path_from_dentry() which takes pre-allocated (probably on stack)
full_path as an argument. This would allow us to avoid allocations
under the spin lock.
--
Best regards,
Pavel Shilovsky