From f5fd4dec3bab362a2ff7844859914e1f191fb905 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Tue, 22 Mar 2011 16:56:35 +0000 Subject: [PATCH] [settings] Formalise notion of setting applicability Expose a function setting_applies() to allow a caller to determine whether or not a particular setting is applicable to a particular settings block. Restrict DHCP-backed settings blocks to accepting only DHCP-based settings. Restrict network device settings blocks to accepting only DHCP-based settings and network device-specific settings such as "mac". Inspired-by: Glenn Brown Signed-off-by: Michael Brown --- src/core/nvo.c | 14 +++++++++ src/core/settings.c | 37 +++++++++++++++++++----- src/drivers/net/phantom/phantom.c | 25 +++++++++++++--- src/include/ipxe/dhcpopts.h | 1 + src/include/ipxe/net80211.h | 21 ++++++++++++++ src/include/ipxe/netdevice.h | 32 +++++++++++++++++++++ src/include/ipxe/settings.h | 40 +++++++++++++++++++++++--- src/interface/smbios/smbios_settings.c | 20 +++++++++++-- src/net/80211/net80211.c | 3 ++ src/net/dhcpopts.c | 12 ++++++++ src/net/dhcppkt.c | 29 +++++++++++++++++++ src/net/netdev_settings.c | 18 ++++++++++++ 12 files changed, 235 insertions(+), 17 deletions(-) diff --git a/src/core/nvo.c b/src/core/nvo.c index f4da407aa..ea58badef 100644 --- a/src/core/nvo.c +++ b/src/core/nvo.c @@ -184,6 +184,19 @@ static int nvo_save ( struct nvo_block *nvo ) { return 0; } +/** + * Check applicability of NVO setting + * + * @v settings Settings block + * @v setting Setting + * @ret applies Setting applies within this settings block + */ +static int nvo_applies ( struct settings *settings __unused, + struct setting *setting ) { + + return dhcpopt_applies ( setting->tag ); +} + /** * Store value of NVO setting * @@ -236,6 +249,7 @@ static int nvo_fetch ( struct settings *settings, struct setting *setting, /** NVO settings operations */ static struct settings_operations nvo_settings_operations = { + .applies = nvo_applies, .store = nvo_store, .fetch = nvo_fetch, }; diff --git a/src/core/settings.c b/src/core/settings.c index 139addd52..a080904c5 100644 --- a/src/core/settings.c +++ b/src/core/settings.c @@ -492,6 +492,19 @@ void unregister_settings ( struct settings *settings ) { ****************************************************************************** */ +/** + * Check applicability of setting + * + * @v settings Settings block + * @v setting Setting + * @ret applies Setting applies within this settings block + */ +int setting_applies ( struct settings *settings, struct setting *setting ) { + + return ( settings->op->applies ? + settings->op->applies ( settings, setting ) : 1 ); +} + /** * Store value of setting * @@ -509,6 +522,10 @@ int store_setting ( struct settings *settings, struct setting *setting, if ( ! settings ) settings = &settings_root; + /* Fail if tag does not apply to this settings block */ + if ( ! setting_applies ( settings, setting ) ) + return -ENOTTY; + /* Sanity check */ if ( ! settings->op->store ) return -ENOTSUP; @@ -564,10 +581,12 @@ int fetch_setting ( struct settings *settings, struct setting *setting, if ( ! settings->op->fetch ) return -ENOTSUP; - /* Try this block first */ - if ( ( ret = settings->op->fetch ( settings, setting, - data, len ) ) >= 0 ) + /* Try this block first, if applicable */ + if ( setting_applies ( settings, setting ) && + ( ( ret = settings->op->fetch ( settings, setting, + data, len ) ) >= 0 ) ) { return ret; + } /* Recurse into each child block in turn */ list_for_each_entry ( child, &settings->children, siblings ) { @@ -886,17 +905,19 @@ struct setting * find_setting ( const char *name ) { /** * Parse setting name as tag number * + * @v settings Settings block * @v name Name * @ret tag Tag number, or 0 if not a valid number */ -static unsigned int parse_setting_tag ( const char *name ) { +static unsigned int parse_setting_tag ( struct settings *settings, + const char *name ) { char *tmp = ( ( char * ) name ); unsigned int tag = 0; while ( 1 ) { tag = ( ( tag << 8 ) | strtoul ( tmp, &tmp, 0 ) ); if ( *tmp == 0 ) - return tag; + return ( tag | settings->tag_magic ); if ( *tmp != '.' ) return 0; tmp++; @@ -946,6 +967,7 @@ parse_setting_name ( const char *name, char *setting_name; char *type_name; struct setting *named_setting; + unsigned int tag; /* Set defaults */ *settings = &settings_root; @@ -979,9 +1001,10 @@ parse_setting_name ( const char *name, if ( ( named_setting = find_setting ( setting_name ) ) != NULL ) { /* Matches a defined named setting; use that setting */ memcpy ( setting, named_setting, sizeof ( *setting ) ); - } else if ( ( setting->tag = parse_setting_tag ( setting_name ) ) !=0){ + } else if ( ( tag = parse_setting_tag ( *settings, + setting_name ) ) != 0 ) { /* Is a valid numeric tag; use the tag */ - setting->tag |= (*settings)->tag_magic; + setting->tag = tag; } else { /* Use the arbitrary name */ setting->name = setting_name; diff --git a/src/drivers/net/phantom/phantom.c b/src/drivers/net/phantom/phantom.c index 1060103c7..a55319ea6 100644 --- a/src/drivers/net/phantom/phantom.c +++ b/src/drivers/net/phantom/phantom.c @@ -1697,6 +1697,24 @@ phantom_clp_setting ( struct phantom_nic *phantom, struct setting *setting ) { return 0; } +/** + * Check applicability of Phantom CLP setting + * + * @v settings Settings block + * @v setting Setting + * @ret applies Setting applies within this settings block + */ +static int phantom_setting_applies ( struct settings *settings, + struct setting *setting ) { + struct phantom_nic *phantom = + container_of ( settings, struct phantom_nic, settings ); + unsigned int clp_setting; + + /* Find Phantom setting equivalent to iPXE setting */ + clp_setting = phantom_clp_setting ( phantom, setting ); + return ( clp_setting != 0 ); +} + /** * Store Phantom CLP setting * @@ -1716,8 +1734,7 @@ static int phantom_store_setting ( struct settings *settings, /* Find Phantom setting equivalent to iPXE setting */ clp_setting = phantom_clp_setting ( phantom, setting ); - if ( ! clp_setting ) - return -ENOTSUP; + assert ( clp_setting != 0 ); /* Store setting */ if ( ( rc = phantom_clp_store ( phantom, phantom->port, @@ -1750,8 +1767,7 @@ static int phantom_fetch_setting ( struct settings *settings, /* Find Phantom setting equivalent to iPXE setting */ clp_setting = phantom_clp_setting ( phantom, setting ); - if ( ! clp_setting ) - return -ENOTSUP; + assert ( clp_setting != 0 ); /* Fetch setting */ if ( ( read_len = phantom_clp_fetch ( phantom, phantom->port, @@ -1767,6 +1783,7 @@ static int phantom_fetch_setting ( struct settings *settings, /** Phantom CLP settings operations */ static struct settings_operations phantom_settings_operations = { + .applies = phantom_setting_applies, .store = phantom_store_setting, .fetch = phantom_fetch_setting, }; diff --git a/src/include/ipxe/dhcpopts.h b/src/include/ipxe/dhcpopts.h index 8fb3d2d79..c5af5d749 100644 --- a/src/include/ipxe/dhcpopts.h +++ b/src/include/ipxe/dhcpopts.h @@ -28,6 +28,7 @@ struct dhcp_options { int ( * realloc ) ( struct dhcp_options *options, size_t len ); }; +extern int dhcpopt_applies ( unsigned int tag ); extern int dhcpopt_store ( struct dhcp_options *options, unsigned int tag, const void *data, size_t len ); extern int dhcpopt_fetch ( struct dhcp_options *options, unsigned int tag, diff --git a/src/include/ipxe/net80211.h b/src/include/ipxe/net80211.h index d70eb7cb8..ac844b408 100644 --- a/src/include/ipxe/net80211.h +++ b/src/include/ipxe/net80211.h @@ -1183,4 +1183,25 @@ static inline u16 net80211_cts_duration ( struct net80211_device *dev, net80211_duration ( dev, size, dev->rates[dev->rate] ) ); } +/** 802.11 device setting tag magic */ +#define NET80211_SETTING_TAG_MAGIC 0x8211 + +/** + * Construct 802.11 setting tag + * + * @v id Unique identifier + * @ret tag Setting tag + */ +#define NET80211_SETTING_TAG( id ) \ + NETDEV_SETTING_TAG ( ( NET80211_SETTING_TAG_MAGIC << 8 ) | (id) ) + +/** SSID setting tag */ +#define NET80211_SETTING_TAG_SSID NET80211_SETTING_TAG ( 0x01 ) + +/** Active scanning setting tag */ +#define NET80211_SETTING_TAG_ACTIVE_SCAN NET80211_SETTING_TAG ( 0x02 ) + +/** Wireless key setting tag */ +#define NET80211_SETTING_TAG_KEY NET80211_SETTING_TAG ( 0x03 ) + #endif diff --git a/src/include/ipxe/netdevice.h b/src/include/ipxe/netdevice.h index ac7cec521..e49191f4a 100644 --- a/src/include/ipxe/netdevice.h +++ b/src/include/ipxe/netdevice.h @@ -390,6 +390,38 @@ struct net_driver { /** Declare a network driver */ #define __net_driver __table_entry ( NET_DRIVERS, 01 ) +/** Network device setting tag magic + * + * All DHCP option settings are deemed to be valid as network device + * settings. There are also some extra non-DHCP settings (such as + * "mac"), which are marked as being valid network device settings by + * using a magic tag value. + */ +#define NETDEV_SETTING_TAG_MAGIC 0xeb + +/** + * Construct network device setting tag + * + * @v id Unique identifier + * @ret tag Setting tag + */ +#define NETDEV_SETTING_TAG( id ) ( ( NETDEV_SETTING_TAG_MAGIC << 24 ) | (id) ) + +/** + * Check if tag is a network device setting tag + * + * @v tag Setting tag + * @ret is_ours Tag is a network device setting tag + */ +#define IS_NETDEV_SETTING_TAG( tag ) \ + ( ( (tag) >> 24 ) == NETDEV_SETTING_TAG_MAGIC ) + +/** MAC address setting tag */ +#define NETDEV_SETTING_TAG_MAC NETDEV_SETTING_TAG ( 0x01 ) + +/** Bus ID setting tag */ +#define NETDEV_SETTING_TAG_BUS_ID NETDEV_SETTING_TAG ( 0x02 ) + extern struct list_head net_devices; extern struct net_device_operations null_netdev_operations; extern struct settings_operations netdev_settings_operations; diff --git a/src/include/ipxe/settings.h b/src/include/ipxe/settings.h index d99e5ec0b..b2b63f8ad 100644 --- a/src/include/ipxe/settings.h +++ b/src/include/ipxe/settings.h @@ -33,7 +33,31 @@ struct setting { * address, etc.). */ struct setting_type *type; - /** DHCP option number, if applicable */ + /** Setting tag, if applicable + * + * The setting tag is a numerical description of the setting + * (such as a DHCP option number, or an SMBIOS structure and + * field number). + * + * Users can construct tags for settings that are not + * explicitly known to iPXE using the generic syntax for + * numerical settings. For example, the setting name "60" + * will be interpreted as referring to DHCP option 60 (the + * vendor class identifier). + * + * This creates a potential for namespace collisions, since + * the interpretation of the numerical description will vary + * according to the settings block. When a user attempts to + * fetch a generic numerical setting, we need to ensure that + * only the intended settings block interprets the numerical + * description. (For example, we do not want to attempt to + * retrieve the subnet mask from SMBIOS, or the system UUID + * from DHCP.) + * + * This potential problem is resolved by allowing the setting + * tag to include a "magic" value indicating the + * interpretation to be placed upon the numerical description. + */ unsigned int tag; }; @@ -45,6 +69,14 @@ struct setting { /** Settings block operations */ struct settings_operations { + /** Check applicability of setting + * + * @v settings Settings block + * @v setting Setting + * @ret applies Setting applies within this settings block + */ + int ( * applies ) ( struct settings *settings, + struct setting *setting ); /** Store value of setting * * @v settings Settings block @@ -84,9 +116,7 @@ struct settings { /** Tag magic * * This value will be ORed in to any numerical tags - * constructed by parse_setting_name(), and can be used to - * avoid e.g. attempting to retrieve the subnet mask from - * SMBIOS, or the system UUID from DHCP. + * constructed by parse_setting_name(). */ unsigned int tag_magic; /** Parent settings block */ @@ -181,6 +211,8 @@ extern int register_settings ( struct settings *settings, struct settings *parent, const char *name ); extern void unregister_settings ( struct settings *settings ); +extern int setting_applies ( struct settings *settings, + struct setting *setting ); extern int store_setting ( struct settings *settings, struct setting *setting, const void *data, size_t len ); extern int fetch_setting ( struct settings *settings, struct setting *setting, diff --git a/src/interface/smbios/smbios_settings.c b/src/interface/smbios/smbios_settings.c index bb7c18dd8..6594c94b6 100644 --- a/src/interface/smbios/smbios_settings.c +++ b/src/interface/smbios/smbios_settings.c @@ -63,6 +63,22 @@ FILE_LICENCE ( GPL2_OR_LATER ); ( (_type) << 16 ) | \ ( offsetof ( _structure, _field ) << 8 ) ) +/** + * Check applicability of SMBIOS setting + * + * @v settings Settings block + * @v setting Setting + * @ret applies Setting applies within this settings block + */ +static int smbios_applies ( struct settings *settings __unused, + struct setting *setting ) { + unsigned int tag_magic; + + /* Check tag magic */ + tag_magic = ( setting->tag >> 24 ); + return ( tag_magic == SMBIOS_TAG_MAGIC ); +} + /** * Fetch value of SMBIOS setting * @@ -87,8 +103,7 @@ static int smbios_fetch ( struct settings *settings __unused, tag_type = ( ( setting->tag >> 16 ) & 0xff ); tag_offset = ( ( setting->tag >> 8 ) & 0xff ); tag_len = ( setting->tag & 0xff ); - if ( tag_magic != SMBIOS_TAG_MAGIC ) - return -ENOENT; + assert ( tag_magic == SMBIOS_TAG_MAGIC ); /* Find SMBIOS structure */ if ( ( rc = find_smbios_structure ( tag_type, &structure ) ) != 0 ) @@ -119,6 +134,7 @@ static int smbios_fetch ( struct settings *settings __unused, /** SMBIOS settings operations */ static struct settings_operations smbios_settings_operations = { + .applies = smbios_applies, .fetch = smbios_fetch, }; diff --git a/src/net/80211/net80211.c b/src/net/80211/net80211.c index d39958ba7..c42928e8a 100644 --- a/src/net/80211/net80211.c +++ b/src/net/80211/net80211.c @@ -206,6 +206,7 @@ struct setting net80211_ssid_setting __setting = { .name = "ssid", .description = "802.11 SSID (network name)", .type = &setting_type_string, + .tag = NET80211_SETTING_TAG_SSID, }; /** Whether to use active scanning @@ -218,6 +219,7 @@ struct setting net80211_active_setting __setting = { .name = "active-scan", .description = "Use an active scan during 802.11 association", .type = &setting_type_int8, + .tag = NET80211_SETTING_TAG_ACTIVE_SCAN, }; /** The cryptographic key to use @@ -230,6 +232,7 @@ struct setting net80211_key_setting __setting = { .name = "key", .description = "Encryption key for protected 802.11 networks", .type = &setting_type_string, + .tag = NET80211_SETTING_TAG_KEY, }; /** @} */ diff --git a/src/net/dhcpopts.c b/src/net/dhcpopts.c index f04b8e712..249fde151 100644 --- a/src/net/dhcpopts.c +++ b/src/net/dhcpopts.c @@ -347,6 +347,18 @@ static int set_dhcp_option ( struct dhcp_options *options, unsigned int tag, return offset; } +/** + * Check applicability of DHCP option setting + * + * @v tag Setting tag number + * @ret applies Setting applies to this option block + */ +int dhcpopt_applies ( unsigned int tag ) { + + return ( tag && ( tag <= DHCP_ENCAP_OPT ( DHCP_MAX_OPTION, + DHCP_MAX_OPTION ) ) ); +} + /** * Store value of DHCP option setting * diff --git a/src/net/dhcppkt.c b/src/net/dhcppkt.c index 237c3e2cf..0a0e458fc 100644 --- a/src/net/dhcppkt.c +++ b/src/net/dhcppkt.c @@ -134,6 +134,19 @@ find_dhcp_packet_field ( unsigned int tag ) { return NULL; } +/** + * Check applicability of DHCP setting + * + * @v dhcppkt DHCP packet + * @v tag Setting tag number + * @ret applies Setting applies within this settings block + */ +static int dhcppkt_applies ( struct dhcp_packet *dhcppkt __unused, + unsigned int tag ) { + + return dhcpopt_applies ( tag ); +} + /** * Store value of DHCP packet setting * @@ -204,6 +217,21 @@ int dhcppkt_fetch ( struct dhcp_packet *dhcppkt, unsigned int tag, * */ +/** + * Check applicability of DHCP setting + * + * @v settings Settings block + * @v setting Setting + * @ret applies Setting applies within this settings block + */ +static int dhcppkt_settings_applies ( struct settings *settings, + struct setting *setting ) { + struct dhcp_packet *dhcppkt = + container_of ( settings, struct dhcp_packet, settings ); + + return dhcppkt_applies ( dhcppkt, setting->tag ); +} + /** * Store value of DHCP setting * @@ -242,6 +270,7 @@ static int dhcppkt_settings_fetch ( struct settings *settings, /** DHCP settings operations */ static struct settings_operations dhcppkt_settings_operations = { + .applies = dhcppkt_settings_applies, .store = dhcppkt_settings_store, .fetch = dhcppkt_settings_fetch, }; diff --git a/src/net/netdev_settings.c b/src/net/netdev_settings.c index f641d3b21..2ff4ad3e3 100644 --- a/src/net/netdev_settings.c +++ b/src/net/netdev_settings.c @@ -22,6 +22,7 @@ FILE_LICENCE ( GPL2_OR_LATER ); #include #include #include +#include #include #include #include @@ -37,13 +38,29 @@ struct setting mac_setting __setting = { .name = "mac", .description = "MAC address", .type = &setting_type_hex, + .tag = NETDEV_SETTING_TAG_MAC, }; struct setting busid_setting __setting = { .name = "busid", .description = "Bus ID", .type = &setting_type_hex, + .tag = NETDEV_SETTING_TAG_BUS_ID, }; +/** + * Check applicability of network device setting + * + * @v settings Settings block + * @v setting Setting + * @ret applies Setting applies within this settings block + */ +static int netdev_applies ( struct settings *settings __unused, + struct setting *setting ) { + + return ( IS_NETDEV_SETTING_TAG ( setting->tag ) || + dhcpopt_applies ( setting->tag ) ); +} + /** * Store value of network device setting * @@ -114,6 +131,7 @@ static void netdev_clear ( struct settings *settings ) { /** Network device configuration settings operations */ struct settings_operations netdev_settings_operations = { + .applies = netdev_applies, .store = netdev_store, .fetch = netdev_fetch, .clear = netdev_clear,