* [PATCH] [v2] libxc: Replace alloca() with mmap() for pfn array sizes greater than a page in linux_privcmd_map_foreign_bulk()
@ 2012-04-20 18:38 Aravindh Puthiyaparambil
2012-04-21 8:31 ` Ian Campbell
0 siblings, 1 reply; 3+ messages in thread
From: Aravindh Puthiyaparambil @ 2012-04-20 18:38 UTC (permalink / raw)
To: xen-devel; +Cc: Andres Lagar-Cavilla, Ian.Campbell
When mapping in large amounts of pages (in the GB range) from a guest in to Dom0 using xc_map_foreign_bulk(), a segfault occurs in the libxc client application. This is because the pfn array in linux_privcmd_map_foreign_bulk() is being allocated using alloca() and the subsequent memcpy causes the stack to blow. This patch replaces the alloca() with mmap() for pfn array sizes greater than a page.
Fix an error print with the correct function name.
Signed-off-by: Aravindh Puthiyaparambil <aravindh@virtuata.com>
Acked-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
diff -r 7c777cb8f705 -r e62ab14d44af tools/libxc/xc_linux_osdep.c
--- a/tools/libxc/xc_linux_osdep.c Wed Apr 18 16:49:55 2012 +0100
+++ b/tools/libxc/xc_linux_osdep.c Fri Apr 20 11:36:02 2012 -0700
@@ -39,6 +39,7 @@
#include "xenctrl.h"
#include "xenctrlosdep.h"
+#define ROUNDUP(_x,_w) (((unsigned long)(_x)+(1UL<<(_w))-1) & ~((1UL<<(_w))-1))
#define ERROR(_m, _a...) xc_osdep_log(xch,XTL_ERROR,XC_INTERNAL_ERROR,_m , ## _a )
#define PERROR(_m, _a...) xc_osdep_log(xch,XTL_ERROR,XC_INTERNAL_ERROR,_m \
" (%d = %s)", ## _a , errno, xc_strerror(xch, errno))
@@ -258,7 +259,7 @@ static void *linux_privcmd_map_foreign_b
fd, 0);
if ( addr == MAP_FAILED )
{
- PERROR("xc_map_foreign_batch: mmap failed");
+ PERROR("xc_map_foreign_bulk: mmap failed");
return NULL;
}
@@ -286,7 +287,21 @@ static void *linux_privcmd_map_foreign_b
* IOCTL_PRIVCMD_MMAPBATCH.
*/
privcmd_mmapbatch_t ioctlx;
- xen_pfn_t *pfn = alloca(num * sizeof(*pfn));
+ xen_pfn_t *pfn;
+ unsigned int pfn_arr_size = ROUNDUP((num * sizeof(*pfn)), XC_PAGE_SHIFT);
+
+ if ( pfn_arr_size <= XC_PAGE_SIZE )
+ pfn = alloca(num * sizeof(*pfn));
+ else
+ {
+ pfn = mmap(NULL, pfn_arr_size, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANON | MAP_POPULATE, -1, 0);
+ if ( pfn == MAP_FAILED )
+ {
+ PERROR("xc_map_foreign_bulk: mmap of pfn array failed");
+ return NULL;
+ }
+ }
memcpy(pfn, arr, num * sizeof(*arr));
@@ -328,6 +343,9 @@ static void *linux_privcmd_map_foreign_b
break;
}
+ if ( pfn_arr_size > XC_PAGE_SIZE )
+ munmap(pfn, pfn_arr_size);
+
if ( rc == -ENOENT && i == num )
rc = 0;
else if ( rc )
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] [v2] libxc: Replace alloca() with mmap() for pfn array sizes greater than a page in linux_privcmd_map_foreign_bulk()
2012-04-20 18:38 [PATCH] [v2] libxc: Replace alloca() with mmap() for pfn array sizes greater than a page in linux_privcmd_map_foreign_bulk() Aravindh Puthiyaparambil
@ 2012-04-21 8:31 ` Ian Campbell
2012-04-23 19:18 ` Aravindh Puthiyaparambil
0 siblings, 1 reply; 3+ messages in thread
From: Ian Campbell @ 2012-04-21 8:31 UTC (permalink / raw)
To: Aravindh Puthiyaparambil; +Cc: xen-devel, Andres Lagar-Cavilla
On Fri, 2012-04-20 at 19:38 +0100, Aravindh Puthiyaparambil wrote:
> When mapping in large amounts of pages (in the GB range) from a guest in to Dom0 using xc_map_foreign_bulk(), a segfault occurs in the libxc client application. This is because the pfn array in linux_privcmd_map_foreign_bulk() is being allocated using alloca() and the subsequent memcpy causes the stack to blow. This patch replaces the alloca() with mmap() for pfn array sizes greater than a page.
>
> Fix an error print with the correct function name.
>
> Signed-off-by: Aravindh Puthiyaparambil <aravindh@virtuata.com>
Thanks. Did you rule out this bug at the usage of alloca() in
linux_gnttab_grant_map? If not then I think that should be changed too.
> Acked-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
>
> diff -r 7c777cb8f705 -r e62ab14d44af tools/libxc/xc_linux_osdep.c
> --- a/tools/libxc/xc_linux_osdep.c Wed Apr 18 16:49:55 2012 +0100
> +++ b/tools/libxc/xc_linux_osdep.c Fri Apr 20 11:36:02 2012 -0700
> @@ -39,6 +39,7 @@
> #include "xenctrl.h"
> #include "xenctrlosdep.h"
>
> +#define ROUNDUP(_x,_w) (((unsigned long)(_x)+(1UL<<(_w))-1) & ~((1UL<<(_w))-1))
> #define ERROR(_m, _a...) xc_osdep_log(xch,XTL_ERROR,XC_INTERNAL_ERROR,_m , ## _a )
> #define PERROR(_m, _a...) xc_osdep_log(xch,XTL_ERROR,XC_INTERNAL_ERROR,_m \
> " (%d = %s)", ## _a , errno, xc_strerror(xch, errno))
> @@ -258,7 +259,7 @@ static void *linux_privcmd_map_foreign_b
> fd, 0);
> if ( addr == MAP_FAILED )
> {
> - PERROR("xc_map_foreign_batch: mmap failed");
> + PERROR("xc_map_foreign_bulk: mmap failed");
> return NULL;
> }
>
> @@ -286,7 +287,21 @@ static void *linux_privcmd_map_foreign_b
> * IOCTL_PRIVCMD_MMAPBATCH.
> */
> privcmd_mmapbatch_t ioctlx;
> - xen_pfn_t *pfn = alloca(num * sizeof(*pfn));
> + xen_pfn_t *pfn;
> + unsigned int pfn_arr_size = ROUNDUP((num * sizeof(*pfn)), XC_PAGE_SHIFT);
> +
> + if ( pfn_arr_size <= XC_PAGE_SIZE )
> + pfn = alloca(num * sizeof(*pfn));
> + else
> + {
> + pfn = mmap(NULL, pfn_arr_size, PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANON | MAP_POPULATE, -1, 0);
> + if ( pfn == MAP_FAILED )
> + {
> + PERROR("xc_map_foreign_bulk: mmap of pfn array failed");
> + return NULL;
> + }
> + }
>
> memcpy(pfn, arr, num * sizeof(*arr));
>
> @@ -328,6 +343,9 @@ static void *linux_privcmd_map_foreign_b
> break;
> }
>
> + if ( pfn_arr_size > XC_PAGE_SIZE )
> + munmap(pfn, pfn_arr_size);
> +
> if ( rc == -ENOENT && i == num )
> rc = 0;
> else if ( rc )
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] [v2] libxc: Replace alloca() with mmap() for pfn array sizes greater than a page in linux_privcmd_map_foreign_bulk()
2012-04-21 8:31 ` Ian Campbell
@ 2012-04-23 19:18 ` Aravindh Puthiyaparambil
0 siblings, 0 replies; 3+ messages in thread
From: Aravindh Puthiyaparambil @ 2012-04-23 19:18 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, Andres Lagar-Cavilla
On Sat, Apr 21, 2012 at 1:31 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Fri, 2012-04-20 at 19:38 +0100, Aravindh Puthiyaparambil wrote:
>> When mapping in large amounts of pages (in the GB range) from a guest in to Dom0 using xc_map_foreign_bulk(), a segfault occurs in the libxc client application. This is because the pfn array in linux_privcmd_map_foreign_bulk() is being allocated using alloca() and the subsequent memcpy causes the stack to blow. This patch replaces the alloca() with mmap() for pfn array sizes greater than a page.
>>
>> Fix an error print with the correct function name.
>>
>> Signed-off-by: Aravindh Puthiyaparambil <aravindh@virtuata.com>
>
> Thanks. Did you rule out this bug at the usage of alloca() in
> linux_gnttab_grant_map? If not then I think that should be changed too.
Yes, you right. It could happen with linux_gnttab_grant_map() also. I
will fix that and resubmit.
Thanks,
Aravindh
>> Acked-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
>>
>> diff -r 7c777cb8f705 -r e62ab14d44af tools/libxc/xc_linux_osdep.c
>> --- a/tools/libxc/xc_linux_osdep.c Wed Apr 18 16:49:55 2012 +0100
>> +++ b/tools/libxc/xc_linux_osdep.c Fri Apr 20 11:36:02 2012 -0700
>> @@ -39,6 +39,7 @@
>> #include "xenctrl.h"
>> #include "xenctrlosdep.h"
>>
>> +#define ROUNDUP(_x,_w) (((unsigned long)(_x)+(1UL<<(_w))-1) & ~((1UL<<(_w))-1))
>> #define ERROR(_m, _a...) xc_osdep_log(xch,XTL_ERROR,XC_INTERNAL_ERROR,_m , ## _a )
>> #define PERROR(_m, _a...) xc_osdep_log(xch,XTL_ERROR,XC_INTERNAL_ERROR,_m \
>> " (%d = %s)", ## _a , errno, xc_strerror(xch, errno))
>> @@ -258,7 +259,7 @@ static void *linux_privcmd_map_foreign_b
>> fd, 0);
>> if ( addr == MAP_FAILED )
>> {
>> - PERROR("xc_map_foreign_batch: mmap failed");
>> + PERROR("xc_map_foreign_bulk: mmap failed");
>> return NULL;
>> }
>>
>> @@ -286,7 +287,21 @@ static void *linux_privcmd_map_foreign_b
>> * IOCTL_PRIVCMD_MMAPBATCH.
>> */
>> privcmd_mmapbatch_t ioctlx;
>> - xen_pfn_t *pfn = alloca(num * sizeof(*pfn));
>> + xen_pfn_t *pfn;
>> + unsigned int pfn_arr_size = ROUNDUP((num * sizeof(*pfn)), XC_PAGE_SHIFT);
>> +
>> + if ( pfn_arr_size <= XC_PAGE_SIZE )
>> + pfn = alloca(num * sizeof(*pfn));
>> + else
>> + {
>> + pfn = mmap(NULL, pfn_arr_size, PROT_READ | PROT_WRITE,
>> + MAP_PRIVATE | MAP_ANON | MAP_POPULATE, -1, 0);
>> + if ( pfn == MAP_FAILED )
>> + {
>> + PERROR("xc_map_foreign_bulk: mmap of pfn array failed");
>> + return NULL;
>> + }
>> + }
>>
>> memcpy(pfn, arr, num * sizeof(*arr));
>>
>> @@ -328,6 +343,9 @@ static void *linux_privcmd_map_foreign_b
>> break;
>> }
>>
>> + if ( pfn_arr_size > XC_PAGE_SIZE )
>> + munmap(pfn, pfn_arr_size);
>> +
>> if ( rc == -ENOENT && i == num )
>> rc = 0;
>> else if ( rc )
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-04-23 19:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-20 18:38 [PATCH] [v2] libxc: Replace alloca() with mmap() for pfn array sizes greater than a page in linux_privcmd_map_foreign_bulk() Aravindh Puthiyaparambil
2012-04-21 8:31 ` Ian Campbell
2012-04-23 19:18 ` Aravindh Puthiyaparambil
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.