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

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


  • To: Juergen Gross <jgross@xxxxxxxx>, Julien Grall <julien@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • Date: Thu, 23 Jun 2022 12:41:49 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=kCpTq1JNCOQ+Xyus0Jrgcs5/tVCqsWepHnlfpIKqwbY=; b=Z3iX6d7LGQjiUqe5YH0dfVkyIV5T7ovHLYJh3vlSauYiwLECeQ+DVX8MF1JiRNAwxjCeYRI/jhEjwxxu0JZa3HMrfY8bN8rUnm8ARkDCezg5nr+g07UBDV9U1ya9SPK3210DesmSZcFJsNKKVa3NpAs2dFJIG4XuVTGuDnTO54MrrEu8uHYb3OYjRtbBfMcufXviBbCaIApAGfcnireMLYFJBXKS3Y51fTpv7K+zLsE5ugDKKLCtp8WscydBXMVmY5HEOALbQFyhzefmkXx80VmlKABiBefJudqff/GdDslFykknWIQmK1yQEaGZpfsBvo3eRuX6fYzT3czXCrg/TA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=lGJQnt5oYkj+pJw7xbHD2kPkGZTJhGtOUf1Za++q2Icj8RakF5tFY6+VOUDBEV1hPGim+hKy628GXYr+I3tjpI8TkXG2kS3oNgCs3jT6AApSsPfenAcFHGLCdiEpRjNxwWMso/TAVQZM1Z5rgFVV5x3wpjBEguKN0ROI7UxQirUIKKHfWMrsbzdJDm7MOub0nIEk+uG1vwkxQnRVTg/Ku0z1G3h2rw/BQdVqGIyji4Kh7Yb6go7gMtlWPknvXxA42iPrQ2WHpExcJ/hREpy45YN/W3WuWaW/vtiAwalN7gLSRJN+DSImd44Lis6oVVOEwofjcRSNFIH/uSVsxNiz6Q==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Julien Grall <jgrall@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Anthony Perard <anthony.perard@xxxxxxxxxx>
  • Delivery-date: Thu, 23 Jun 2022 12:42:15 +0000
  • Ironport-data: A9a23:CBdLl6IOCViAmOeCFE+Rw5QlxSXFcZb7ZxGr2PjKsXjdYENSg2YPx 2oaCz3UO/mKNGqjKY9wYdzi/UkEsJ/QztFqTAVlqX01Q3x08seUXt7xwmUcns+xwm8vaGo9s q3yv/GZdJhcokf0/0vrav67xZVF/fngqoDUUYYoAQgsA14+IMsdoUg7wbRh3NQz2YHR7z6l4 rseneWOYDdJ5BYsWo4kw/rrRMRH5amaVJsw5zTSVNgT1LPsvyB94KE3fMldG0DQUIhMdtNWc s6YpF2PEsE1yD92Yj+tuu6TnkTn2dc+NyDW4pZdc/DKbhSvOkXee0v0XRYRQR4/ttmHozx+4 IxrkISyRzsuAojnpbQHbwRbFw49HrITrdcrIVDn2SCS52vvViK1ht5JVQQxN4Be/ftrC2ZT8 /BeMCoKch2Im+OxxvS8V/VogcMgasLsOevzuFk5lW2fUalgHM6FGvuajTNb9G5YasRmNPDSf ccGLxFoawzNeUZnMVYLEpMu2uyvgxETdhUH8w/E+PppuwA/yiQrgZHGKcbWYObVXJ8Ok16G/ mjm4l3QV0Ry2Nu3jGDtHmiXru3AhyTgQ6oJCaa1sPVthTW7xHEXCRAQfUu2p7++kEHWc8JSL QkY9zQjqYA29Ve3VZ/tUhugunmGsxUAHd1KHIUS6g6Xw67Qyw+cD3oDSHhKb9lOnNAybSwn0 BmOhdyBONB0mLicSHbY/bDNqzq3YHERNTVbO39CShYZ6d7+po11lgjIUttoDK+yiJvyBC30x DeJ6iM5gt3/kPI26klyxnif6xrEm3QDZlddCtn/No590j5EWQ==
  • Ironport-hdrordr: A9a23:fK3qi6/GIdqnPnBSAfRuk+AiI+orL9Y04lQ7vn2ZKSY5TiVXra CTdZUgpHvJYVMqMk3I9uruBEDtex3hHP1OkOws1NWZLWrbUQKTRekP0WKL+Vbd8kbFh4xgPM lbEpSXCLfLfCVHZcSR2njFLz73quP3j5xBho3lvglQpRkBUdAG0+/gYDzraXGfQmN9dPwEPa vZ3OVrjRy6d08aa8yqb0N1JdQq97Xw5evbiQdtPW9e1DWz
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHYhvPL6UjRJR/HKUCr4NOCggNq961c2uKAgAAUbIA=
  • Thread-topic: [PATCH] tools/xenstored: Harden corrupt()

On 23/06/2022 12:28, Juergen Gross wrote:
> 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");

str ?: "ENOMEM"

seeing as we use this idiom in a lot of places.

~Andrew

 


Rackspace

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