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

Re: [PATCH] tools/xl: don't crash on NULL command line


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Jason Andryuk <jason.andryuk@xxxxxxx>
  • Date: Mon, 28 Jul 2025 20:29:19 -0400
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=citrix.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; 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=kmdBH9afn7zJDj1WMqCUFB9PfGkWN6VL+CLvyskocpk=; b=vFdh7msP3q+Bl06dfxRTPQEKYh/cOZBj3lDla9ck9LFpP5a8cakL+ehjCZgbJEbugUAtnvt3CaAXnWTCiVfCcP3tGkpvMGJq4+3oYAHTWkscAzdeLd0n5Qf0Byd9WjDkpZghT0b5dyERlf51BvJy3fTrs0G3tF6J/Pn9fkQ2dWZQTK31MW4WaTvR+VewPp9OIfoixePD2eTv+gJ132Hvy/J3vtLgxBhmv/D+eJ1Ssp9ru6nHMjm8p0KG99coS5Jqv0Q5NiXJpLYfGoo4qX9JK6oYWN6EXXxNbGmj3yt001OTpuJ2KzKmYtYeZjLlsa0dbs53CCyaw1xt8xaJY85vUg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=FFsqX+wyiiE8UYLsndV0dzBFReyOAd9H83u2wH6Gu+QE8tx6OULYCujcjt0sGIrKGK6bot259Id3+qNsLAurtUOSdDMoFElFmqjIjMBwqO4MIEMDD38cFv4riO3JSrKMXf8Mo0YyKN9xNoADkc40dbit6enFUJEqNhtRse9+LlNtM09WRsAGt6xgVSwF+k1UrzYCsep8Ie1Vqt74T83BZsQliDuMdwYam2cINQISnKJUW1qvP9RaBIXKj0fWX1kkKeRvE3lEDwrVvxEk5dOqc5AcfT1TmxFw7vVRRsW9kUZcr0SRRXgYRsPevgvpBD8Jw+Kbmrs+eg4dzXD8kC/asg==
  • Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>
  • Delivery-date: Tue, 29 Jul 2025 00:29:52 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 2025-07-28 06:45, Andrew Cooper wrote:
On 28/07/2025 11:24 am, Marek Marczykowski-Górecki wrote:
When running xl in a domU, it doesn't have access to the Xen command
line. Before the non-truncating xc_xenver_cmdline(), it was always set
with strdup, possibly of an empty string. Now it's NULL. Treat it the
same as empty cmdline, as it was before. Autoballoon isn't relevant for
xl devd in a domU anyway.

Fixes: 75f91607621c ("tools: Introduce a non-truncating xc_xenver_cmdline()")
Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
---
So, apparently the "No API/ABI change" was a lie... it changed "empty
string" to NULL in libxl_version_info->commandline. Quick search didn't
spot any other (in-tree) place that could trip on NULL there. IMO NULL
value in this case makes more sense. Buf maybe for the API stability
reasons the old behavior should be restored?

Hmm, I didn't intend to change things, but I also didn't anticipate
libxl__strdup()'s behaviour, or for something to depend on that.

I think it isn't strdup()'s behaviour, but rather the old code:

-    xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline);
-    info->commandline = libxl__strdup(NOGC, u.xen_commandline);
+    info->commandline = xc_xenver_commandline(ctx->xch);

No error checking on xc_version(), so strdup() is duplicating whatever (stale?) data may be in the union.

Regards,
Jason

While this does turn out to be a marginal API change, I think your
solution is the right one.  I do not think it's reasonable for there to
be one magic pointer that has differing NULL-ness to the others, and
NULL for "no information" is the better interface.




 


Rackspace

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