>From d9bf88a24518afa96fe55beda040a3605b81610a Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Fri, 8 Nov 2024 17:07:16 +0200 Subject: [PATCH] pack_fields() changes Remove ability to declare packed field array in ascending and descending order, over just ascending. Actually implement the overlap check in modpost. The simple check: "elem.endbit <= previous_elem.startbit" enforces both ascending order and non-overlapping checks. Move the check that fields don't exceed the buffer size inside the pack_fields() and unpack_fields() macro. With the assumption that the packed field array is sorted (enforced by modpost), we only need to check this for the last element of the array. Remove the packed buffer size from the DECLARE_PACKED_FIELDS_*() macros, and thus from the .rodata.packed_fields_* sections. Also remove the modpost code that looks for those sizes. Modify the pack_fields() API to always expect a strong type as the "pbuf" argument, because we take sizeof(*pbuf) as the packed buffer size. Remove the pbuflen from the required argument list, and adapt ice accordingly. Signed-off-by: Vladimir Oltean --- drivers/net/ethernet/intel/ice/ice_common.c | 16 +-- include/linux/packing.h | 32 +++-- include/linux/packing_types.h | 15 +- scripts/mod/packed_fields.c | 148 ++++++-------------- 4 files changed, 76 insertions(+), 135 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c index 98845d790791..631963427082 100644 --- a/drivers/net/ethernet/intel/ice/ice_common.c +++ b/drivers/net/ethernet/intel/ice/ice_common.c @@ -1381,7 +1381,7 @@ static void ice_copy_rxq_ctx_to_hw(struct ice_hw *hw, PACKED_FIELD((lsb) + (width) - 1, (lsb), struct struct_name, struct_field) /* LAN Rx Queue Context */ -DECLARE_PACKED_FIELDS_S(ice_rlan_ctx_fields, ICE_RXQ_CTX_SZ) = { +static const DECLARE_PACKED_FIELDS_S(ice_rlan_ctx_fields) = { /* Field Width LSB */ ICE_CTX_STORE(ice_rlan_ctx, head, 13, 0), ICE_CTX_STORE(ice_rlan_ctx, cpuid, 8, 13), @@ -1416,10 +1416,8 @@ DECLARE_PACKED_FIELDS_S(ice_rlan_ctx_fields, ICE_RXQ_CTX_SZ) = { static void ice_pack_rxq_ctx(const struct ice_rlan_ctx *ctx, ice_rxq_ctx_buf_t *buf) { - BUILD_BUG_ON(sizeof(*buf) != ICE_RXQ_CTX_SZ); - - pack_fields(buf, sizeof(*buf), ctx, ice_rlan_ctx_fields, - QUIRK_LITTLE_ENDIAN | QUIRK_LSW32_IS_FIRST); + pack_fields(buf, ctx, ice_rlan_ctx_fields, QUIRK_LITTLE_ENDIAN | + QUIRK_LSW32_IS_FIRST); } /** @@ -1448,7 +1446,7 @@ int ice_write_rxq_ctx(struct ice_hw *hw, struct ice_rlan_ctx *rlan_ctx, } /* LAN Tx Queue Context */ -DECLARE_PACKED_FIELDS_S(ice_tlan_ctx_fields, ICE_TXQ_CTX_SZ) = { +static const DECLARE_PACKED_FIELDS_S(ice_tlan_ctx_fields) = { /* Field Width LSB */ ICE_CTX_STORE(ice_tlan_ctx, base, 57, 0), ICE_CTX_STORE(ice_tlan_ctx, port_num, 3, 57), @@ -1489,10 +1487,8 @@ DECLARE_PACKED_FIELDS_S(ice_tlan_ctx_fields, ICE_TXQ_CTX_SZ) = { */ void ice_pack_txq_ctx(const struct ice_tlan_ctx *ctx, ice_txq_ctx_buf_t *buf) { - BUILD_BUG_ON(sizeof(*buf) != ICE_TXQ_CTX_SZ); - - pack_fields(buf, sizeof(*buf), ctx, ice_tlan_ctx_fields, - QUIRK_LITTLE_ENDIAN | QUIRK_LSW32_IS_FIRST); + pack_fields(buf, ctx, ice_tlan_ctx_fields, QUIRK_LITTLE_ENDIAN | + QUIRK_LSW32_IS_FIRST); } /* Sideband Queue command wrappers */ diff --git a/include/linux/packing.h b/include/linux/packing.h index b66c4809e121..d4988131e13a 100644 --- a/include/linux/packing.h +++ b/include/linux/packing.h @@ -35,16 +35,26 @@ void unpack_fields_m(const void *pbuf, size_t pbuflen, void *ustruct, const struct packed_field_m *fields, size_t num_fields, u8 quirks); -#define pack_fields(pbuf, pbuflen, ustruct, fields, quirks) \ - _Generic((fields), \ - const struct packed_field_s * : pack_fields_s, \ - const struct packed_field_m * : pack_fields_m \ - )(pbuf, pbuflen, ustruct, fields, ARRAY_SIZE(fields), quirks) - -#define unpack_fields(pbuf, pbuflen, ustruct, fields, quirks) \ - _Generic((fields), \ - const struct packed_field_s * : unpack_fields_s, \ - const struct packed_field_m * : unpack_fields_m \ - )(pbuf, pbuflen, ustruct, fields, ARRAY_SIZE(fields), quirks) +#define pack_fields(pbuf, ustruct, fields, quirks) \ + ({ typeof(fields[0]) *__f = fields; \ + size_t pbuflen = sizeof(*pbuf); \ + size_t num_fields = ARRAY_SIZE(fields); \ + BUILD_BUG_ON(__f[num_fields - 1].startbit >= BITS_PER_BYTE * pbuflen); \ + _Generic(__f, \ + const struct packed_field_s * : pack_fields_s, \ + const struct packed_field_m * : pack_fields_m \ + )(pbuf, pbuflen, ustruct, __f, num_fields, quirks); \ + }) + +#define unpack_fields(pbuf, ustruct, fields, quirks) \ + ({ typeof(fields[0]) *__f = fields; \ + size_t pbuflen = sizeof(*pbuf); \ + size_t num_fields = ARRAY_SIZE(fields); \ + BUILD_BUG_ON(__f[num_fields - 1].startbit >= BITS_PER_BYTE * pbuflen); \ + _Generic(__f, \ + const struct packed_field_s * : unpack_fields_s, \ + const struct packed_field_m * : unpack_fields_m \ + )(pbuf, pbuflen, ustruct, __f, num_fields, quirks); \ + }) #endif diff --git a/include/linux/packing_types.h b/include/linux/packing_types.h index 49ae4329a494..c761d93be235 100644 --- a/include/linux/packing_types.h +++ b/include/linux/packing_types.h @@ -24,12 +24,6 @@ struct packed_field_s { GEN_PACKED_FIELD_MEMBERS(u8); }; -#define __pf_section_s __section(".rodata.packed_fields_s") - -#define DECLARE_PACKED_FIELDS_S(name, buffer_sz) \ - const size_t __ ## name ## _buffer_sz __pf_section_s = buffer_sz; \ - const struct packed_field_s name[] __pf_section_s - /* Medium packed field. Use with bit offsets < 65536, buffers < 8KB and * unpacked structures < 64KB. */ @@ -37,11 +31,10 @@ struct packed_field_m { GEN_PACKED_FIELD_MEMBERS(u16); }; -#define __pf_section_m __section(".rodata.packed_fields_m") - -#define DECLARE_PACKED_FIELDS_M(name, buffer_sz) \ - const size_t __ ## name ## _buffer_sz __pf_section_m = buffer_sz; \ - const struct packed_field_m name[] __pf_section_m +#define DECLARE_PACKED_FIELDS_S(name) \ + struct packed_field_s name[] __section(".rodata.packed_fields_s") +#define DECLARE_PACKED_FIELDS_M(name) \ + struct packed_field_m name[] __section(".rodata.packed_fields_m") #define PACKED_FIELD(start, end, struct_name, struct_field) \ { \ diff --git a/scripts/mod/packed_fields.c b/scripts/mod/packed_fields.c index aa9dbd704e52..5927b0c7f404 100644 --- a/scripts/mod/packed_fields.c +++ b/scripts/mod/packed_fields.c @@ -87,18 +87,18 @@ struct field_symbol { size_t data_size; void *data; struct module *mod; - char *name; + const char *name; }; static HASHTABLE_DEFINE(field_hashtable, 1U << 10); -static struct field_symbol *alloc_field(char *name, struct module *mod) +static struct field_symbol *alloc_field(const char *name, struct module *mod) { struct field_symbol *f = xmalloc(sizeof(*f)); memset(f, 0, sizeof(*f)); f->mod = mod; - f->name = name; + f->name = xstrdup(name); return f; } @@ -124,10 +124,8 @@ void handle_packed_field_symbol(struct module *mod, struct elf_info *info, { unsigned int secindex = get_secindex(info, sym); enum field_type type = UNKNOWN_SECTION; - bool is_size_symbol = false; struct field_symbol *f; const char *section; - char *name; /* Skip symbols without a name */ if (*symname == '\0') @@ -148,18 +146,9 @@ void handle_packed_field_symbol(struct module *mod, struct elf_info *info, if (type == UNKNOWN_SECTION) return; - name = xstrdup(symname); - - /* Extract original field name from the size symbol */ - if (!fnmatch("__*_buffer_sz", name, 0)) { - name += strlen("__"); - name[strlen(name) - strlen("_buffer_sz")] = '\0'; - is_size_symbol = true; - } - - f = find_field(name, mod); + f = find_field(symname, mod); if (!f) { - f = alloc_field(name, mod); + f = alloc_field(symname, mod); f->type = type; hash_add_field(f); } @@ -170,118 +159,71 @@ void handle_packed_field_symbol(struct module *mod, struct elf_info *info, return; } - if (is_size_symbol) { - size_t *size_data = sym_get_data(info, sym); - size_t size = TO_NATIVE(*size_data); - - if (f->buffer_size && f->buffer_size != size) { - error("%s has buffer size %zu, but symbol %s says the size should be %zu\n", - f->name, f->buffer_size, symname, size); - } - - f->buffer_size = size; - } else { - if (f->data) - error("%s has multiple data symbols???\n", - f->name); - - f->data_size = sym->st_size; - f->data = xmalloc(f->data_size); - memcpy(f->data, sym_get_data(info, sym), f->data_size); + if (f->data) { + error("%s has multiple data symbols???\n", + f->name); } -} -enum element_order { - FIRST_ELEMENT, - SECOND_ELEMENT, - ASCENDING_ORDER, - DESCENDING_ORDER, -}; + f->data_size = sym->st_size; + f->data = xmalloc(f->data_size); + memcpy(f->data, sym_get_data(info, sym), f->data_size); +} static void check_packed_field_array(const struct field_symbol *f) { - struct packed_field_elem previous_elem = {}; size_t field_size = field_type_to_size(f->type); - enum element_order order = FIRST_ELEMENT; + struct packed_field_elem previous_elem = {}; void *data_ptr; - int count; + size_t count; /* check that the data is a multiple of the size */ - if (f->data_size % field_size != 0) { - error("symbol %s of module %s has size %zu which is not a multiple of the field size (%zu)\n", - f->name, f->mod->name, f->data_size, field_size); + if (f->data_size % field_size) { + error("[%s] symbol %s has size %zu which is not a multiple of the field size (%zu)\n", + f->mod->name, f->name, f->data_size, field_size); return; } - data_ptr = f->data; - count = 0; - - while (data_ptr < f->data + f->data_size) { + for (data_ptr = f->data, count = 0; + data_ptr < f->data + f->data_size; + data_ptr += field_size, count++) { struct packed_field_elem elem = {}; get_field_contents(data_ptr, f->type, &elem); - if (elem.startbit < elem.endbit) - error("\"%s\" [%s.ko] element %u startbit (%" PRIu64 ") must be larger than endbit (%" PRIu64 ")\n", - f->name, f->mod->name, count, elem.startbit, - elem.endbit); + if (elem.size != 1 && elem.size != 2 && + elem.size != 4 && elem.size != 8) { + error("[%s] \"%s\" field %zu unpacked size (%" PRIu64 + ") must be 1, 2, 4, or 8 bytes\n", + f->mod->name, f->name, count, elem.size); + } - if (elem.startbit >= BITS_PER_BYTE * f->buffer_size) - error("\"%s\" [%s.ko] element %u startbit (%" PRIu64 ") puts field outsize of the packed buffer size (%" PRIu64 ")\n", - f->name, f->mod->name, count, elem.startbit, - f->buffer_size); + if (elem.startbit < elem.endbit) { + error("[%s] \"%s\" field %zu (%" PRIu64 "-%" PRIu64 + "): start bit must be >= end bit\n", + f->mod->name, f->name, count, elem.startbit, + elem.endbit); + } - if (elem.startbit - elem.endbit >= BITS_PER_BYTE * elem.size) - error("\"%s\" [%s.ko] element %u startbit (%" PRIu64 ") and endbit (%" PRIu64 ") indicate a field of width (%" PRIu64 ") which does not fit into the field size (%" PRIu64 ")\n", - f->name, f->mod->name, count, elem.startbit, + if (elem.startbit - elem.endbit >= BITS_PER_BYTE * elem.size) { + error("[%s] \"%s\" field %zu (%" PRIu64 "-%" PRIu64 + ") has a width of %" PRIu64 + " which does not fit into the unpacked structure field (%" + PRIu64 " bytes)\n", + f->mod->name, f->name, count, elem.startbit, elem.endbit, elem.startbit - elem.endbit, elem.size); - - if (elem.size != 1 && elem.size != 2 && elem.size != 4 && elem.size != 8) - error("\"%s\" [%s.ko] element %u size (%" PRIu64 ") must be 1, 2, 4, or 8\n", - f->name, f->mod->name, count, elem.size); - - switch (order) { - case FIRST_ELEMENT: - order = SECOND_ELEMENT; - break; - case SECOND_ELEMENT: - order = previous_elem.startbit < elem.startbit ? - ASCENDING_ORDER : DESCENDING_ORDER; - break; - default: - break; } - switch (order) { - case ASCENDING_ORDER: - if (previous_elem.startbit >= elem.startbit) - error("\"%s\" [%s.ko] element %u startbit (%" PRIu64 ") expected to be arranged in ascending order, but previous element startbit is %" PRIu64 "\n", - f->name, f->mod->name, count, - elem.startbit, previous_elem.startbit); - if (previous_elem.endbit >= elem.endbit) - error("\"%s\" [%s.ko] element %u endbit (%" PRIu64 ") expected to be arranged in ascending order, but previous element endbit is %" PRIu64 "\n", - f->name, f->mod->name, count, elem.endbit, - previous_elem.endbit); - - break; - case DESCENDING_ORDER: - if (previous_elem.startbit <= elem.startbit) - error("\"%s\" [%s.ko] element %u startbit (%" PRIu64 ") expected to be arranged in descending order, but previous element startbit is %" PRIu64 "\n", - f->name, f->mod->name, count, - elem.startbit, previous_elem.startbit); - if (previous_elem.endbit <= elem.endbit) - error("\"%s\" [%s.ko] element %u endbit (%" PRIu64 ") expected to be arranged in descending order, but previous element endbit is %" PRIu64 "\n", - f->name, f->mod->name, count, - elem.endbit, previous_elem.endbit); - break; - default: - break; + if (count && elem.endbit <= previous_elem.startbit) { + error("[%s] \"%s\" field %zu (%" PRIu64 "-%" PRIu64 + ") overlaps with previous field (%" PRIu64 "-%" PRIu64 + ") or array is not declared in ascending order\n", + f->mod->name, f->name, count, elem.startbit, + elem.endbit, previous_elem.startbit, + previous_elem.endbit); } previous_elem = elem; - data_ptr += field_size; - count++; } } -- 2.43.0