|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v8 1/2] libxc: Report consistent errors in xc_resource_op
Le 02/03/2026 à 16:14, Anthony PERARD a écrit :
> On Fri, Feb 27, 2026 at 05:00:05PM +0000, Teddy Astie wrote:
>> xc_report_op doesn't update errno in some error conditions.
>> Make sure it reports -ENOMEM in out of memory errors and -EINVAL
>> in invalid usages errors.
>>
>> Signed-off-by: Teddy Astie <teddy.astie@xxxxxxxxxx>
>> ---
>> v7: Introduced
>> v8: Use errno to report errors
>>
>> tools/libs/ctrl/xc_resource.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/tools/libs/ctrl/xc_resource.c b/tools/libs/ctrl/xc_resource.c
>> index cb6a97202b..ac1524d1bd 100644
>> --- a/tools/libs/ctrl/xc_resource.c
>> +++ b/tools/libs/ctrl/xc_resource.c
>> @@ -28,7 +28,10 @@ static int xc_resource_op_one(xc_interface *xch,
>> xc_resource_op_t *op)
>> XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
>>
>> if ( xc_hypercall_bounce_pre(xch, entries) )
>> + {
>> + errno = ENOMEM;
>
> Looking at xc_hypercall_bounce_pre(), it's looks like `errno` should
> already be set. On Linux, that would be `mmap()` or `madvise()` updating
> it.
>
>> return -1;
>> + }
>>
>> platform_op.cmd = XENPF_resource_op;
>> platform_op.u.resource_op.nr_entries = op->nr_entries;
>> @@ -54,11 +57,15 @@ static int xc_resource_op_multi(xc_interface *xch,
>> uint32_t nr_ops, xc_resource_
>> call_list = xc_hypercall_buffer_alloc(xch, call_list,
>> sizeof(*call_list) * nr_ops);
>> if ( !call_list )
>> + {
>> + errno = ENOMEM;
>
> Here, xc_hypercall_buffer_alloc() should already have updated `errno`.
> (It's a function called by xc_hypercall_bounce_pre(), so we've got the
> same culprit updating `errno`.)
>
>> return -1;
>> + }
>>
>> platform_ops = xc_hypercall_buffer_array_create(xch, nr_ops);
>> if ( !platform_ops )
>> {
>> + errno = ENOMEM;
>
> Here, xc_hypercall_buffer_array_create() calls `calloc()` and `malloc()`
> which will update `errno`.
>
>> rc = -1;
>> goto out;
>> }
>> @@ -66,6 +73,7 @@ static int xc_resource_op_multi(xc_interface *xch,
>> uint32_t nr_ops, xc_resource_
>> entries_list = xc_hypercall_buffer_array_create(xch, nr_ops);
>> if ( !entries_list )
>> {
>> + errno = ENOMEM;
>
> Same as above.
>
>> rc = -1;
>> goto out;
>> }
>> @@ -81,6 +89,7 @@ static int xc_resource_op_multi(xc_interface *xch,
>> uint32_t nr_ops, xc_resource_
>> platform_op, sizeof(xen_platform_op_t));
>> if ( !platform_op )
>> {
>> + errno = ENOMEM;
>
> With xc_hypercall_buffer_array_alloc(), `errno` is updated by `mmap` or
> `madvise`, like the first case.
>
>> rc = -1;
>> goto out;
>> }
>> @@ -90,6 +99,7 @@ static int xc_resource_op_multi(xc_interface *xch,
>> uint32_t nr_ops, xc_resource_
>> entries, entries_size);
>> if ( !entries)
>> {
>> + errno = ENOMEM;
>
> Same as above.
>
Ok for all, though good to note that malloc ones are documented as
platform specific behaviors (at least in malloc(3)).
But in our case (POSIX platforms), it's supposed to be the case.
>> rc = -1;
>> goto out;
>> }
>> @@ -137,6 +147,7 @@ int xc_resource_op(xc_interface *xch, uint32_t nr_ops,
>> xc_resource_op_t *ops)
>> if ( nr_ops > 1 )
>> return xc_resource_op_multi(xch, nr_ops, ops);
>>
>> + errno = EINVAL;
>
> Ha! This one was missing indeed.
>
> The patch description will need to be updated with all the chunk be one
> been dropped.
>
Overall, it's
libxc: Report EINVAL on incorrect xc_resource_op usage
When passing 0 operations to xc_resource_op, a error is reported without
setting errno appropriately.
> Thanks,
>
>
> --
> Anthony Perard | Vates XCP-ng Developer
>
> XCP-ng & Xen Orchestra - Vates solutions
>
> web: https://vates.tech
>
>
--
Teddy Astie | Vates XCP-ng Developer
XCP-ng & Xen Orchestra - Vates solutions
web: https://vates.tech
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |