* [Qemu-devel] [PATCH] hw/9pfs: Use O_NOFOLLOW when opening files on server
@ 2013-05-22 11:22 Aneesh Kumar K.V
2013-05-22 14:54 ` Stefan Hajnoczi
0 siblings, 1 reply; 3+ messages in thread
From: Aneesh Kumar K.V @ 2013-05-22 11:22 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, Aneesh Kumar K.V
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
9p server should never follow a symlink. So use O_NOFOLLOW with all open
syscall
Tested-by: "M. Mohan Kumar" <mohan@in.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
hw/9pfs/virtio-9p-handle.c | 2 +-
hw/9pfs/virtio-9p-local.c | 48 +++++++++++++++++++++++++++++++++++++++-------
2 files changed, 42 insertions(+), 8 deletions(-)
diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c
index fe8e0ed..e2a89e3 100644
--- a/hw/9pfs/virtio-9p-handle.c
+++ b/hw/9pfs/virtio-9p-handle.c
@@ -608,7 +608,7 @@ static int handle_init(FsContext *ctx)
struct file_handle fh;
struct handle_data *data = g_malloc(sizeof(struct handle_data));
- data->mountfd = open(ctx->fs_root, O_DIRECTORY);
+ data->mountfd = open(ctx->fs_root, O_DIRECTORY | O_NOFOLLOW);
if (data->mountfd < 0) {
ret = data->mountfd;
goto err_out;
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 87aa75d..fc93e9e 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -59,6 +59,33 @@ static const char *local_mapped_attr_path(FsContext *ctx,
return buffer;
}
+static FILE *local_fopen(const char *path, const char *mode)
+{
+ int fd, o_mode = 0;
+ FILE *fp;
+ int flags = O_NOFOLLOW;
+ /*
+ * only supports two modes
+ */
+ if (mode[0] == 'r') {
+ flags |= O_RDONLY;
+ } else if (mode[0] == 'w') {
+ flags |= O_WRONLY | O_TRUNC | O_CREAT;
+ o_mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
+ } else {
+ return NULL;
+ }
+ fd = open(path, flags, o_mode);
+ if (fd == -1) {
+ return NULL;
+ }
+ fp = fdopen(fd, mode);
+ if (!fp) {
+ close(fd);
+ }
+ return fp;
+}
+
#define ATTR_MAX 100
static void local_mapped_file_attr(FsContext *ctx, const char *path,
struct stat *stbuf)
@@ -68,7 +95,7 @@ static void local_mapped_file_attr(FsContext *ctx, const char *path,
char attr_path[PATH_MAX];
local_mapped_attr_path(ctx, path, attr_path);
- fp = fopen(attr_path, "r");
+ fp = local_fopen(attr_path, "r");
if (!fp) {
return;
}
@@ -152,7 +179,7 @@ static int local_set_mapped_file_attr(FsContext *ctx,
char attr_path[PATH_MAX];
int uid = -1, gid = -1, mode = -1, rdev = -1;
- fp = fopen(local_mapped_attr_path(ctx, path, attr_path), "r");
+ fp = local_fopen(local_mapped_attr_path(ctx, path, attr_path), "r");
if (!fp) {
goto create_map_file;
}
@@ -179,7 +206,7 @@ create_map_file:
}
update_map_file:
- fp = fopen(attr_path, "w");
+ fp = local_fopen(attr_path, "w");
if (!fp) {
ret = -1;
goto err_out;
@@ -316,7 +343,7 @@ static int local_open(FsContext *ctx, V9fsPath *fs_path,
char buffer[PATH_MAX];
char *path = fs_path->data;
- fs->fd = open(rpath(ctx, path, buffer), flags);
+ fs->fd = open(rpath(ctx, path, buffer), flags | O_NOFOLLOW);
return fs->fd;
}
@@ -601,6 +628,11 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
V9fsString fullname;
char buffer[PATH_MAX];
+ /*
+ * Mark all the open to not follow symlinks
+ */
+ flags |= O_NOFOLLOW;
+
v9fs_string_init(&fullname);
v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
path = fullname.data;
@@ -676,8 +708,9 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath,
if (fs_ctx->export_flags & V9FS_SM_MAPPED) {
int fd;
ssize_t oldpath_size, write_size;
- fd = open(rpath(fs_ctx, newpath, buffer), O_CREAT|O_EXCL|O_RDWR,
- SM_LOCAL_MODE_BITS);
+ fd = open(rpath(fs_ctx, newpath, buffer),
+ O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW,
+ SM_LOCAL_MODE_BITS);
if (fd == -1) {
err = fd;
goto out;
@@ -705,7 +738,8 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath,
} else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
int fd;
ssize_t oldpath_size, write_size;
- fd = open(rpath(fs_ctx, newpath, buffer), O_CREAT|O_EXCL|O_RDWR,
+ fd = open(rpath(fs_ctx, newpath, buffer),
+ O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW,
SM_LOCAL_MODE_BITS);
if (fd == -1) {
err = fd;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/9pfs: Use O_NOFOLLOW when opening files on server
2013-05-22 11:22 [Qemu-devel] [PATCH] hw/9pfs: Use O_NOFOLLOW when opening files on server Aneesh Kumar K.V
@ 2013-05-22 14:54 ` Stefan Hajnoczi
2013-05-23 8:24 ` Aneesh Kumar K.V
0 siblings, 1 reply; 3+ messages in thread
From: Stefan Hajnoczi @ 2013-05-22 14:54 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: aliguori, qemu-devel
On Wed, May 22, 2013 at 04:52:54PM +0530, Aneesh Kumar K.V wrote:
> diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c
> index fe8e0ed..e2a89e3 100644
> --- a/hw/9pfs/virtio-9p-handle.c
> +++ b/hw/9pfs/virtio-9p-handle.c
> @@ -608,7 +608,7 @@ static int handle_init(FsContext *ctx)
> struct file_handle fh;
> struct handle_data *data = g_malloc(sizeof(struct handle_data));
>
> - data->mountfd = open(ctx->fs_root, O_DIRECTORY);
> + data->mountfd = open(ctx->fs_root, O_DIRECTORY | O_NOFOLLOW);
Why is the root path not allowed to be a symlink?
And if so, it would be more user-friendly to resolve the path before
open. That way we don't need to bug the user with an error here.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/9pfs: Use O_NOFOLLOW when opening files on server
2013-05-22 14:54 ` Stefan Hajnoczi
@ 2013-05-23 8:24 ` Aneesh Kumar K.V
0 siblings, 0 replies; 3+ messages in thread
From: Aneesh Kumar K.V @ 2013-05-23 8:24 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: aliguori, qemu-devel
Stefan Hajnoczi <stefanha@gmail.com> writes:
> On Wed, May 22, 2013 at 04:52:54PM +0530, Aneesh Kumar K.V wrote:
>> diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c
>> index fe8e0ed..e2a89e3 100644
>> --- a/hw/9pfs/virtio-9p-handle.c
>> +++ b/hw/9pfs/virtio-9p-handle.c
>> @@ -608,7 +608,7 @@ static int handle_init(FsContext *ctx)
>> struct file_handle fh;
>> struct handle_data *data = g_malloc(sizeof(struct handle_data));
>>
>> - data->mountfd = open(ctx->fs_root, O_DIRECTORY);
>> + data->mountfd = open(ctx->fs_root, O_DIRECTORY | O_NOFOLLOW);
>
> Why is the root path not allowed to be a symlink?
No specific reason.
>
> And if so, it would be more user-friendly to resolve the path before
> open. That way we don't need to bug the user with an error here.
I will drop that hunk.
-aneesh
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-05-23 8:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-22 11:22 [Qemu-devel] [PATCH] hw/9pfs: Use O_NOFOLLOW when opening files on server Aneesh Kumar K.V
2013-05-22 14:54 ` Stefan Hajnoczi
2013-05-23 8:24 ` Aneesh Kumar K.V
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.