Re: [Qemu-devel] [V3 PATCH 7/8] virtio-9p: Move file post creation changes to none security model

[Venkateswararao Jujjuri (JV)]

On 1/20/2011 1:45 PM, Stefan Hajnoczi wrote:
> On Thu, Jan 20, 2011 at 9:15 PM, Venkateswararao Jujjuri (JV)
> <address@hidden> wrote:
>> On 1/20/2011 12:59 AM, Stefan Hajnoczi wrote:
>>> On Tue, Jan 18, 2011 at 01:54:16PM +0530, M. Mohan Kumar wrote:
>>>> After creating a file object, its permission and ownership details are 
>>>> updated
>>>> as per client's request for both passthrough and none security model. But 
>>>> with
>>>> chrooted environment its not required for passthrough security model. Move 
>>>> all
>>>> post file creation changes to none security model
>>>>
>>>> Signed-off-by: M. Mohan Kumar <address@hidden>
>>>> ---
>>>>  hw/9pfs/virtio-9p-local.c |   19 ++++++-------------
>>>>  1 files changed, 6 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
>>>> index 08fd67f..d2e32e2 100644
>>>> --- a/hw/9pfs/virtio-9p-local.c
>>>> +++ b/hw/9pfs/virtio-9p-local.c
>>>> @@ -208,21 +208,14 @@ static int local_set_xattr(const char *path, FsCred 
>>>> *credp)
>>>>      return 0;
>>>>  }
>>>>
>>>> -static int local_post_create_passthrough(FsContext *fs_ctx, const char 
>>>> *path,
>>>> +static int local_post_create_none(FsContext *fs_ctx, const char *path,
>>>>          FsCred *credp)
>>>>  {
>>>> +    int retval;
>>>>      if (chmod(rpath(fs_ctx, path), credp->fc_mode & 07777) < 0) {
>>>>          return -1;
>>>>      }
>>>> -    if (lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid) < 0) {
>>>> -        /*
>>>> -         * If we fail to change ownership and if we are
>>>> -         * using security model none. Ignore the error
>>>> -         */
>>>> -        if (fs_ctx->fs_sm != SM_NONE) {
>>>> -            return -1;
>>>> -        }
>>>> -    }
>>>> +    retval = lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
>>>>      return 0;
>>>>  }
>>>
>>> retval is unused.
>>>
>>> Can multiple virtio-9p requests execute at a time?  chmod() and lchown()
>>> after creation is a race condition if other requests can execute
>>> concurrently.
>>
>> If some level of serialization is needed it will be done at the client/guest
>> inode level.
>> Are you worried about filesystem semantics? or do you see some corruption if 
>> they
>> get executed in parallel?
> 
> My main concern is unreliable results due to the race conditions
> between creation and the fixups that are performed afterwards.
> 
> Is virtio-9p only useful for single guest exclusive access?  I thought
> both guest and host could access files at the same time?  What about
> multiple VMs sharing a directory?  These scenarios can only work if
> operations are made atomic.

For now, there is only one exploiter for the filesystem. The Guest/client.

In the future it could be different and we 'may' support multiple 
exploiters/users.
Note that we have two security models
1. Passthrough 2. Mapped. (3. None -  can be ignored as it is intended for
developer)

Mapped model is advised when you have only one exploiter;
Passthrough model is for more practical application/uses and it can be
used for multiple exploiters (say guests).

In passthrough model we don't do chmod() lchmod() after creating files.

Thanks,
JV
> 
> Stefan