On Thu, Jan 20, 2011 at 2:48 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
> On Thu, Jan 20, 2011 at 08:11:27PM +0530, M. Mohan Kumar wrote:>> On Thursday 20 January 2011 2:29:54 pm Stefan Hajnoczi wrote:>> > On Tue, Jan 18, 2011 at 01:54:16PM +0530, M. Mohan Kumar wrote:>>>> > > - 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.>> >>>>> That was used to disable the warning message "error: ignoring return value of>> ‘lchown’, declared with attribute warn_unused_result">>>> Otherwise I have to use>> if (lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid)) {>> ;>> }>>>> > Can multiple virtio-9p requests execute at a time? chmod() and lchown()>> > after creation is a race condition if other requests can execute>> > concurrently.>> >>>>> We can't implement file creation with requested user credentials and permission>> bits in the none security model atomically. Its expected behaviour only>> Well you could do the nasty trick of forking a child process> and doing setuid/gid in that and then creating the file before> letting the parent continue.>> if ((pid = fork()) == 0) {> setuid(fc_uid);> setgid(fc_gid);> fd =open("foo", O_CREAT);> close(fd);> } else {> waitpid(pid);> }>> This kind of approach is in fact required if you want to> be able to create files with a special uid/gid on a root> squashing NFS server, because otherwise your QEMU running> as root will have its files squashed to 'nobody' when initially> created, and lchown will fail with EPERM. You might decide> that root squashing NFS is too painful to care about supporting> though :-)
I was thinking about this approach and it's similar to the chroot
helper process, but this time you have a helper process that does
umask/setgid/setuid as necessary. Performance will be bad but there's
really no way around this.
Either implement something that works 90% of the time only but runs a
bit faster or implement something that works all the time but runs
slow. It's not a nice trade-off.
Stefan

On 1/20/2011 1:45 PM, Stefan Hajnoczi wrote:
> On Thu, Jan 20, 2011 at 9:15 PM, Venkateswararao Jujjuri (JV)> <jvrao@linux.vnet.ibm.com> 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 <mohan@in.ibm.com>>>>> --->>>> 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