On 03/03/2014 04:34 PM, Markus Armbruster wrote:
> 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).
>>
>> The related test:
> [...]
>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>> ---
>> hw/9pfs/cofs.c | 15 ++-
>> hw/9pfs/virtio-9p-handle.c | 9 +-
>> hw/9pfs/virtio-9p-local.c | 285
>> +++++++++++++++++++++++++++--------------
>> hw/9pfs/virtio-9p-posix-acl.c | 52 ++++++--
>> hw/9pfs/virtio-9p-xattr-user.c | 27 +++-
>> hw/9pfs/virtio-9p-xattr.c | 9 +-
>> hw/9pfs/virtio-9p-xattr.h | 27 +++-
>> hw/9pfs/virtio-9p.h | 6 +-
>> 8 files changed, 292 insertions(+), 138 deletions(-)
>>
>> diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c
>> index 3891050..739bad0 100644
>> --- a/hw/9pfs/cofs.c
>> +++ b/hw/9pfs/cofs.c
>> @@ -20,18 +20,24 @@
>> 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);
>> v9fs_path_read_lock(s);
>> v9fs_co_run_in_worker(
>> - {
>> + while (1) {
>> len = s->ops->readlink(&s->ctx, path,
>> - buf->data, PATH_MAX - 1);
>> + buf->data, maxlen - 1);
>> + if (len == maxlen - 1) {
>> + g_free(buf->data);
>> + maxlen *= 2;
>> + buf->data = g_malloc(maxlen);
>> + continue;
>> + }
>> if (len > -1) {
>> buf->size = len;
>> buf->data[len] = 0;
> err = 0;
>> } else {
>> err = -errno;
>> }
>> + break;
>> });
>> v9fs_path_unlock(s);
>> if (err) {
>
> Harmless off-by-one: you double the buffer even when the link contents
> plus terminating null fits the buffer exactly (len == maxlen - 1).
>
> I prefer to have the exceptional stuff handled in conditionals, and not
> the normal stuff, like this:
>
> for (;;) {
> len = s->ops->readlink(&s->ctx, path, buf->data, maxlen);
> if (len < 0) {
> err = -errno;
> break;
> }
> if (len == maxlen) {
> g_free(buf->data);
> maxlen *= 2;
> buf->data = g_malloc(maxlen);
> continue;
> }
> buf->size = len;
> buf->data[len] = 0;
> err = 0;
> break;
> }
>
> Matter of taste.
>
That sounds good to me, after this patch pass checking, I will/should
send patch v2 for it.
> [...]
>
> I skimmed a few more hunks, and they look good to me. Leaving full
> review to Aneesh.
>
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
