[PATCH phosphor-host-ipmid] Support for restricted mode for IPMI commands

Jeremy Kerr jk at ozlabs.org
Thu Apr 21 20:08:31 AEST 2016

Hi Tom,

> From: tomjose <tomjoseph at in.ibm.com>

Can you use a full name for the commit, and add some detail to the
commit log?

Also, you may want to check out our contributing guidelines:


> @@ -88,6 +89,12 @@ ipmi_ret_t ipmi_app_set_acpi_power_state(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
>      *data_len = 0;
>      printf("IPMI SET ACPI STATE Ignoring for now\n");
> +
> +    if(restricted_mode)
> +    {
> +    }
> +
>      return rc;
>  }

I don't think this is a maintainable method of implementing restricted
mode, for a couple of reasons:

 - The checks are scattered throughout the codebase. We would have to
   audit every ipmi function to check that restricted mode is
   implemented properly.

 - If we add new commands, we have no way to ensure the whitelist is
   implemented correctly for that command.

We'd be better off implementing the check at a single location, where
the IPMI command is first demultiplexed. This way, we can audit it in a
central location, and have a single list of whitelisted commands.

I'd suggest we have a function, looking something like:

struct {
	ipmi_netfn_t	netfn;
	ipmi_cmd_t	cmd;
} ipmi_whitelist[] = {

bool ipmi_command_is_allowed(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
	void *data)
	int i;

	if (!restricted_mode)
		return true;

	for (i = 0; i < ARRAY_SIZE(ipmi_whitelist); i++) {
		if (netfn == ipmi_whitelist[i].netfn &&
				cmd == ipmi_whitelist[i].cmd)
			return true;

	return false;

this would be called from handle_ipmi_command, before we route it to a
handler; if it returns false there, the we return

This way, we have a whitelist rather than a blacklist, and we can audit
it much more easily.



More information about the openbmc mailing list