2023-10-27 18:24:30

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH v13 15/35] fs: Export anon_inode_getfile_secure() for use by KVM

Export anon_inode_getfile_secure() so that it can be used by KVM to create
and manage file-based guest memory without need a fullblow filesystem.
The "standard" anon_inode_getfd() doesn't work for KVM's use case as KVM
needs a unique inode for each file, e.g. to be able to independently
manage the size and lifecycle of a given file.

Note, KVM doesn't need a "secure" version, just unique inodes, i.e. ignore
the name.

Signed-off-by: Sean Christopherson <[email protected]>
---
fs/anon_inodes.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 24192a7667ed..4190336180ee 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -176,6 +176,7 @@ struct file *anon_inode_getfile_secure(const char *name,
return __anon_inode_getfile(name, fops, priv, flags,
context_inode, true);
}
+EXPORT_SYMBOL_GPL(anon_inode_getfile_secure);

static int __anon_inode_getfd(const char *name,
const struct file_operations *fops,
--
2.42.0.820.g83a721a137-goog


2023-10-30 17:32:02

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH v13 15/35] fs: Export anon_inode_getfile_secure() for use by KVM

On 10/27/23 20:21, Sean Christopherson wrote:
> Export anon_inode_getfile_secure() so that it can be used by KVM to
> create and manage file-based guest memory without need a fullblow

without introducing a full-blown

Otherwise,

Reviewed-by: Paolo Bonzini <[email protected]>

Paolo

> filesystem. The "standard" anon_inode_getfd() doesn't work for KVM's use
> case as KVM needs a unique inode for each file, e.g. to be able to
> independently manage the size and lifecycle of a given file.

2023-11-02 16:24:51

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH v13 15/35] fs: Export anon_inode_getfile_secure() for use by KVM

On Fri, Oct 27, 2023 at 11:21:57AM -0700, Sean Christopherson wrote:
> Export anon_inode_getfile_secure() so that it can be used by KVM to create
> and manage file-based guest memory without need a fullblow filesystem.
> The "standard" anon_inode_getfd() doesn't work for KVM's use case as KVM
> needs a unique inode for each file, e.g. to be able to independently
> manage the size and lifecycle of a given file.
>
> Note, KVM doesn't need a "secure" version, just unique inodes, i.e. ignore
> the name.
>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---

Before we enshrine this misleading name let's rename this to:

create_anon_inode_getfile()

I don't claim it's a great name but it's better than *_secure() which is
very confusing. So just:

struct file *create_anon_inode_getfile(const char *name,
const struct file_operations *fops,
void *priv, int flags)

May also just remove that context_inode argument from the exported
function. The only other caller is io_uring. And neither it nor this
patchset need the context_inode thing afaict. Merge conflict risk is
extremely low so carrying that as part of this patchset is fine and
shouldn't cause huge issues for you.

2023-11-03 10:42:02

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH v13 15/35] fs: Export anon_inode_getfile_secure() for use by KVM

On 11/2/23 17:24, Christian Brauner wrote:
> On Fri, Oct 27, 2023 at 11:21:57AM -0700, Sean Christopherson wrote:
>> Export anon_inode_getfile_secure() so that it can be used by KVM to create
>> and manage file-based guest memory without need a fullblow filesystem.
>> The "standard" anon_inode_getfd() doesn't work for KVM's use case as KVM
>> needs a unique inode for each file, e.g. to be able to independently
>> manage the size and lifecycle of a given file.
>>
>> Note, KVM doesn't need a "secure" version, just unique inodes, i.e. ignore
>> the name.
>>
>> Signed-off-by: Sean Christopherson <[email protected]>
>> ---
>
> Before we enshrine this misleading name let's rename this to:
>
> create_anon_inode_getfile()
>
> I don't claim it's a great name but it's better than *_secure() which is
> very confusing. So just:
>
> struct file *create_anon_inode_getfile(const char *name,
> const struct file_operations *fops,
> void *priv, int flags)

I slightly prefer anon_inode_create_getfile(); grepping include/linux
for '\<create_' vs '_create_' shows that this is much more common.

Neither userfaultfd (which uses anon_inode_getfd_secure()) nor io_uring
strictly speaking need separate inodes; they do want the call to
inode_init_security_anon(). But I agree that the new name is better and
I will adjust the comments so that it is clear why you'd use this
function instead of anon_inode_get{file,fd}().

> May also just remove that context_inode argument from the exported
> function. The only other caller is io_uring. And neither it nor this
> patchset need the context_inode thing afaict.

True, OTOH we might as well rename anon_inode_getfd_secure() to
anon_inode_create_getfd(), and that one does need context_inode.

I'll Cc you on v14 and will carry the patch in my tree.

Paolo

> Merge conflict risk is
> extremely low so carrying that as part of this patchset is fine and
> shouldn't cause huge issues for you.
>