The fix for XSA-417 had a bug: domain_alloc_permrefs() will not return
a negative value in case of an error, but a plain errno value.

Note this is not considered to be a security issue, as the only case
where domain_alloc_permrefs() will return an error is a failed memory
allocation. As a guest should not be able to drive Xenstore out of
memory, this is NOT a problem a guest can trigger at will.

Fixes: ab128218225d ("tools/xenstore: fix checking node permissions")
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/xenstored_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 31a862b715..a1d3047e48 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1784,7 +1784,7 @@ static int do_set_perms(const void *ctx, struct connection *conn,
 	if (!xenstore_strings_to_perms(perms.p, perms.num, permstr))
 		return errno;
 
-	if (domain_alloc_permrefs(&perms) < 0)
+	if (domain_alloc_permrefs(&perms))
 		return ENOMEM;
 	if (perms.p[0].perms & XS_PERM_IGNORE)
 		return ENOENT;
-- 
2.35.3

Hi Juergen,

On 20/07/2023 16:04, Juergen Gross wrote:
> The fix for XSA-417 had a bug: domain_alloc_permrefs() will not return
> a negative value in case of an error, but a plain errno value.
> 
> Note this is not considered to be a security issue, as the only case
> where domain_alloc_permrefs() will return an error is a failed memory
> allocation. As a guest should not be able to drive Xenstore out of
> memory, this is NOT a problem a guest can trigger at will.
> 
> Fixes: ab128218225d ("tools/xenstore: fix checking node permissions")
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

On 21.07.2023 00:34, Julien Grall wrote:
> On 20/07/2023 16:04, Juergen Gross wrote:
>> The fix for XSA-417 had a bug: domain_alloc_permrefs() will not return
>> a negative value in case of an error, but a plain errno value.
>>
>> Note this is not considered to be a security issue, as the only case
>> where domain_alloc_permrefs() will return an error is a failed memory
>> allocation. As a guest should not be able to drive Xenstore out of
>> memory, this is NOT a problem a guest can trigger at will.
>>
>> Fixes: ab128218225d ("tools/xenstore: fix checking node permissions")
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> Acked-by: Julien Grall <jgrall@amazon.com>

In the interest of not missing to add this to my to-be-backported
collection, I've included this in what I've committed just now. It
correcting an earlier XSA fix, I guess we may want to go as far as
backporting this also to the security-only stable trees (i.e.
through to 4.14 rather than just back to 4.16)?

As an aside - note that 4.14 is about to close.

Jan