[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] tools/xenstored: Harden corrupt()



On 23.06.22 13:24, Julien Grall wrote:
From: Julien Grall <jgrall@xxxxxxxxxx>

At the moment, corrupt() is neither checking for allocation failure
nor freeing the allocated memory.

Harden the code by printing ENOMEM if the allocation failed and
free 'str' after the last use.

This is not considered to be a security issue because corrupt() should
only be called when Xenstored thinks the database is corrupted. Note
that the trigger (i.e. a guest reliably provoking the call) would be
a security issue.

Fixes: 06d17943f0cd ("Added a basic integrity checker, and some basic ability to 
recover from store")
Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
---
  tools/xenstore/xenstored_core.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index fa733e714e9a..b6279bdfe229 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -2065,7 +2065,11 @@ void corrupt(struct connection *conn, const char *fmt, 
...)
        va_end(arglist);
log("corruption detected by connection %i: err %s: %s",
-           conn ? (int)conn->id : -1, strerror(saved_errno), str);
+           conn ? (int)conn->id : -1, strerror(saved_errno),
+           str ? str : "ENOMEM");
+
+       if (str)

No need for the "if". talloc_free() handles NULL quite fine.

+               talloc_free(str);
check_store();
  }

With above fixed:

Reviewed-by: Juergen Gross <jgross@xxxxxxxx>


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.