From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:43108)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <zhang.zhanghailiang@huawei.com>) id 1XY7da-0001Fu-Dv
	for qemu-devel@nongnu.org; Sun, 28 Sep 2014 02:07:19 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <zhang.zhanghailiang@huawei.com>) id 1XY7dV-0003hz-Jq
	for qemu-devel@nongnu.org; Sun, 28 Sep 2014 02:07:14 -0400
Message-ID: <5427A560.5040302@huawei.com>
Date: Sun, 28 Sep 2014 14:06:24 +0800
From: zhanghailiang <zhang.zhanghailiang@huawei.com>
MIME-Version: 1.0
References: <1411096150-3044-1-git-send-email-zhang.zhanghailiang@huawei.com>
	<542588D8.30701@redhat.com>
In-Reply-To: <542588D8.30701@redhat.com>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v2] qga: Rewrite code where using readdir_r
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org
Cc: mdroth@linux.vnet.ibm.com, armbru@redhat.com, qemu-stable@nongnu.org, luonengjun@huawei.com, peter.huangpeng@huawei.com, lcapitulino@redhat.com

On 2014/9/26 23:40, Paolo Bonzini wrote:
> Il 19/09/2014 05:09, zhanghailiang ha scritto:
>> If readdir_r fails, error_setg_errno will reference the freed
>> pointer *dirpath*.
>>
>> Moreover, readdir_r may cause a buffer overflow, using readdir instead.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> ---
>>   v2:
>> - Switch readdir_r to readdir (Comment of Eric Blake)
>> ---
>>   qga/commands-posix.c | 27 +++++++++++++++------------
>>   1 file changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>> index 7eed7f4..f6f3e3c 100644
>> --- a/qga/commands-posix.c
>> +++ b/qga/commands-posix.c
>> @@ -956,7 +956,7 @@ static void build_guest_fsinfo_for_virtual_device(char const *syspath,
>>   {
>>       DIR *dir;
>>       char *dirpath;
>> -    struct dirent entry, *result;
>> +    struct dirent *entry;
>>
>>       dirpath = g_strdup_printf("%s/slaves", syspath);
>>       dir = opendir(dirpath);
>> @@ -965,22 +965,24 @@ static void build_guest_fsinfo_for_virtual_device(char const *syspath,
>>           g_free(dirpath);
>>           return;
>>       }
>> -    g_free(dirpath);
>>
>>       for (;;) {
>> -        if (readdir_r(dir, &entry, &result) != 0) {
>> -            error_setg_errno(errp, errno, "readdir_r(\"%s\")", dirpath);
>> -            break;
>> -        }
>> -        if (!result) {
>> +        errno = 0;
>> +        entry = readdir(dir);
>> +        if (entry == NULL) {
>> +            if (errno) {
>> +                error_setg_errno(errp, errno, "readdir(\"%s\")", dirpath);
>> +            }
>>               break;
>>           }
>>
>> -        if (entry.d_type == DT_LNK) {
>> -            g_debug(" slave device '%s'", entry.d_name);
>> -            dirpath = g_strdup_printf("%s/slaves/%s", syspath, entry.d_name);
>> -            build_guest_fsinfo_for_device(dirpath, fs, errp);
>> -            g_free(dirpath);
>> +        if (entry->d_type == DT_LNK) {
>> +            char *path;
>> +
>> +            g_debug(" slave device '%s'", entry->d_name);
>> +            path = g_strdup_printf("%s/slaves/%s", syspath, entry->d_name);
>> +            build_guest_fsinfo_for_device(path, fs, errp);
>> +            g_free(path);
>>
>>               if (*errp) {
>>                   break;
>> @@ -988,6 +990,7 @@ static void build_guest_fsinfo_for_virtual_device(char const *syspath,
>>           }
>>       }
>>
>> +    g_free(dirpath);
>>       closedir(dir);
>>   }
>>
>>
>
> Thanks,
>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Michael Roth will pick this up.
>

OK, Thanks!

> Paolo
>
> .
>