Chen Gang <gang.chen.5...@gmail.com> writes:
> When path is truncated by PATH_MAX limitation, it causes QEMU to access
> incorrect file. So use original full path instead of PATH_MAX within
> 9pfs (need check/process ENOMEM for related memory allocation).
>
> Also find/fix several another related issues when failure occurs.
>
> - check 'fh' in handle_name_to_path().
>
> - need call v9fs_string_free() at 'err_out:' in local_unlinkat().
>
> - sprintf() will cause memory overflow in "virtio-9p-local.c"
Sound like distinct bugs. Have you considered fixing them in separate
patches for easier review and clearer commit messages?
> The related test:
>
> - Environments (for qemu-devel):
>
> - Host is under fedora17 desktop with ext4fs:
>
> qemu-system-x86_64 -hda test.img -m 1024 \
> -net nic,vlan=4,model=virtio,macaddr=00:16:35:AF:94:04 \
> -net tap,vlan=4,ifname=tap4,script=no,downscript=no \
> -device virtio-9p-pci,id=fs0,fsdev=fsdev0,mount_tag=hostshare \
> -fsdev local,security_model=passthrough,id=fsdev0,\
> path=/upstream/vm/data/share/1234567890abcdefghijklmnopqrstuvwxyz\
> ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890acdefghijklmnopqrstuvwxyz\
> ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890/111111111111111111111111111\
> 1111111111111111111111111111111111111111111111111111222222222222\
> 2222222222222222222222222222222222222222222222222222222222222222\
> 2222222222222222222222222222222222233333333333333333333333333333\
> 3333333333333333333333333333333333
>
> - Guest is ubuntu12 server with 9pfs.
>
> mount -t 9p -o trans=virtio,version=9p2000.L hostshare /share
>
> - Limitations:
>
> full path limitation is PATH_MAX (4096B include nul) under Linux.
> file/dir node name maximized length is 256 (include nul) under ext4.
>
> - Special test:
>
> Under host, modify the file: "/upstream/vm/data/share/1234567890abcdefg\
[...]
> ppppppppppppppppppppppppppppppppppppppp/test1234567890file.log"
> (need enter dir firstly, then modify file, or can not open it).
>
> Under guest, still allow modify "test1234567890file.log" (will generate
> "test123456" file with contents).
>
> After apply this patch, can not open "test1234567890file.log" under guest
> (permission denied).
>
>
> - Common test:
>
> All are still OK after apply this path.
>
> "mkdir -p", "create/open file/dir", "modify file/dir", "rm file/dir".
> change various mount point paths under host and/or guest.
>
>
> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
> ---
> hw/9pfs/cofs.c | 22 ++-
> hw/9pfs/virtio-9p-handle.c | 25 ++-
> hw/9pfs/virtio-9p-local.c | 433
> +++++++++++++++++++++++++++++++----------
> hw/9pfs/virtio-9p-posix-acl.c | 76 ++++++--
> hw/9pfs/virtio-9p-xattr-user.c | 36 +++-
> hw/9pfs/virtio-9p-xattr.c | 13 +-
> hw/9pfs/virtio-9p-xattr.h | 39 +++-
> hw/9pfs/virtio-9p.h | 6 +-
> 8 files changed, 504 insertions(+), 146 deletions(-)
>
> diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c
> index 3891050..ba69965 100644
> --- a/hw/9pfs/cofs.c
> +++ b/hw/9pfs/cofs.c
> @@ -20,18 +20,31 @@
> int v9fs_co_readlink(V9fsPDU *pdu, V9fsPath *path, V9fsString *buf)
> {
> int err;
> - ssize_t len;
> + ssize_t len, maxlen = PATH_MAX;
> V9fsState *s = pdu->s;
>
> if (v9fs_request_cancelled(pdu)) {
> return -EINTR;
> }
> - buf->data = g_malloc(PATH_MAX);
> + buf->data = g_malloc(maxlen);
> + if (!buf->data) {
Can't happen, because g_malloc() returns NULL only when its argument is
zero. Many more instances below, also with other memory allocation
functions, such as g_strdup_printf(). Please clean up and resubmit.
> + return -ENOMEM;
> + }
[...]
