Yeah, to verify you'd need a BMC that has the ability to configure a
dynamic IP address.
here's the patch i just commited upstream
https://github.com/chu11/freeipmi-mirror/commit/41d0d70f09b4becfceef0517543cbf335c0e927a
Thanks for the catch
Oh and I got an e-mail that said you subscribed to freeipmi-devel
today? I'm not sure what was up with gnu's mailman, but hopefully it's
fixed now.
Al
On 2/21/24 09:25, Pavel Cahyna wrote:
On Wed, Feb 21, 2024 at 09:04:13AM -0800, Al Chu wrote:
On 2/21/24 03:37, Pavel Cahyna wrote:
[ please Cc: me in replies, I am not subscribed and my attempt to
subscribe hits "Bug in Mailman version 2.1.29" .]
Hello Al,
certain fields are only used for static vs dynamic addresses, so only specific
ones need to be initialized as long as things are programmed correctly.
IIRC, the patch below fixed the mistake that I used "source" instead of
"source_type" for dynamic addresses.
that's my point - the patch below initializes "source_type" and leaves
"source" uninitialized, but then it keeps using "ipv6_data.source". To
me, the patch seems incomplete and
get_dynamic_address_source_type_string (ipv6_data.source)) < 0)
looks like it should be changed to
get_dynamic_address_source_type_string (ipv6_data.source_type)) < 0)
Ohhh, I get it now. I misread the prior e-mail. Yeah, I think you're
right, that doesn't look right at all.
Thanks for the catch. I can get this fixed sometime today.
I assume you hit this bug? Is it critical for a new release?
I have not hit the bug, I am not using ipmi-config myself, actually. The
bug was found by a static code scanner. Do you know how would one
reproduce it or in general test this code? I suppose this needs a BMC
with an IPv6 address configured?
It is not entirely critical, but I would prefer not to update to a
release which has a coding error added. If you have a patch, I will add
it to the CentOS Stream and Fedora packages.
Regards, Pavel
Al
What do you think? If not, I don't see where does ipv6_data.source get
initialized.
Regards, Pavel
On Tue, Feb 20, 2024 at 05:03:46PM +0100, Pavel Cahyna wrote:
Hello,
I have a question about this commit:
commit 478ffaa7d2ffe240a0afaf9d7d5189342c94bd4d
Author: Albert Chu <ch...@llnl.gov>
Date: Wed Jan 26 21:48:47 2022 -0800
ipmi-config: correct IPv6 dynamic address checkout error
It contains this change:
-----
@@ -1089,15 +1090,15 @@ _get_ipv6_dynamic_address (ipmi_config_state_data_t
*state_data,
goto cleanup;
}
- if (FIID_OBJ_GET (obj_cmd_rs, "source", &val) < 0)
+ if (FIID_OBJ_GET (obj_cmd_rs, "source_type", &val) < 0)
{
pstdout_fprintf (state_data->pstate,
stderr,
- "fiid_obj_get: 'source': %s\n",
+ "fiid_obj_get: 'source_type': %s\n",
fiid_obj_errormsg (obj_cmd_rs));
goto cleanup;
}
- ipv6_data->source = val;
+ ipv6_data->source_type = val;
if (fiid_obj_get_data (obj_cmd_rs,
"address",
-----
which, IIUC, leaves ipv6_data->source uninitialized.
It then contains this change:
-----
@@ -1186,7 +1187,7 @@ ipv6_dynamic_address_source_checkout
(ipmi_config_state_data_t *state_data,
if (ipmi_config_section_update_keyvalue_output (state_data,
kv,
-
get_dynamic_address_source_string (ipv6_data.source)) < 0)
+
get_dynamic_address_source_type_string (ipv6_data.source)) < 0)
return (IPMI_CONFIG_ERR_FATAL_ERROR);
rv = IPMI_CONFIG_ERR_SUCCESS;
-----
which keeps using ipv6_data.source, which looks uninitialized. This looks
suspect, but I admit I
don't really understand the code.
Best regards, Pavel
--
Al Chu
Livermore Computing
Lawrence Livermore National Laboratory
--
Al Chu
Livermore Computing
Lawrence Livermore National Laboratory
