| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20240930-packing-kunit-tests-and-split-pack-unpack-v1-5-94b1f04aca85@intel.com>
Date: Mon, 30 Sep 2024 16:19:38 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Andrew Morton <akpm@...ux-foundation.org>,
Vladimir Oltean <olteanv@...il.com>,
"David S. Miller" <davem@...emloft.net>
Cc: netdev@...r.kernel.org, Jacob Keller <jacob.e.keller@...el.com>,
Vladimir Oltean <vladimir.oltean@....com>,
Przemek Kitszel <przemyslaw.kitszel@...el.com>
Subject: [PATCH net-next 05/10] lib: packing: duplicate pack() and unpack()
implementations
From: Vladimir Oltean <vladimir.oltean@....com>
packing() is now used in some hot paths, and it would be good to get rid
of some ifs and buts that depend on "op", to speed things up a little bit.
With the main implementations now taking size_t endbit, we no longer
have to check for negative values. Update the local integer variables to
also be size_t to match.
Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
---
include/linux/packing.h | 4 +-
lib/packing.c | 243 ++++++++++++++++++++++++++++++------------------
2 files changed, 154 insertions(+), 93 deletions(-)
diff --git a/include/linux/packing.h b/include/linux/packing.h
index ea25cb93cc70..5d36dcd06f60 100644
--- a/include/linux/packing.h
+++ b/include/linux/packing.h
@@ -20,8 +20,8 @@ enum packing_op {
int packing(void *pbuf, u64 *uval, int startbit, int endbit, size_t pbuflen,
enum packing_op op, u8 quirks);
-int pack(void *pbuf, const u64 *uval, size_t startbit, size_t endbit,
- size_t pbuflen, u8 quirks);
+int pack(void *pbuf, u64 uval, size_t startbit, size_t endbit, size_t pbuflen,
+ u8 quirks);
int unpack(const void *pbuf, u64 *uval, size_t startbit, size_t endbit,
size_t pbuflen, u8 quirks);
diff --git a/lib/packing.c b/lib/packing.c
index 2922db8a528c..500184530d07 100644
--- a/lib/packing.c
+++ b/lib/packing.c
@@ -9,11 +9,11 @@
#include <linux/types.h>
#include <linux/bitrev.h>
-static void adjust_for_msb_right_quirk(u64 *to_write, int *box_start_bit,
- int *box_end_bit, u8 *box_mask)
+static void adjust_for_msb_right_quirk(u64 *to_write, size_t *box_start_bit,
+ size_t *box_end_bit, u8 *box_mask)
{
- int box_bit_width = *box_start_bit - *box_end_bit + 1;
- int new_box_start_bit, new_box_end_bit;
+ size_t box_bit_width = *box_start_bit - *box_end_bit + 1;
+ size_t new_box_start_bit, new_box_end_bit;
*to_write >>= *box_end_bit;
*to_write = bitrev8(*to_write) >> (8 - box_bit_width);
@@ -48,7 +48,7 @@ static void adjust_for_msb_right_quirk(u64 *to_write, int *box_start_bit,
*
* Return: the physical offset into the buffer corresponding to the logical box.
*/
-static int calculate_box_addr(int box, size_t len, u8 quirks)
+static size_t calculate_box_addr(size_t box, size_t len, u8 quirks)
{
size_t offset_of_group, offset_in_group, this_group = box / 4;
size_t group_size;
@@ -69,13 +69,10 @@ static int calculate_box_addr(int box, size_t len, u8 quirks)
}
/**
- * packing - Convert numbers (currently u64) between a packed and an unpacked
- * format. Unpacked means laid out in memory in the CPU's native
- * understanding of integers, while packed means anything else that
- * requires translation.
+ * pack - Pack u64 number into bitfield of buffer.
*
* @pbuf: Pointer to a buffer holding the packed value.
- * @uval: Pointer to an u64 holding the unpacked value.
+ * @uval: CPU-readable unpacked value to pack.
* @startbit: The index (in logical notation, compensated for quirks) where
* the packed value starts within pbuf. Must be larger than, or
* equal to, endbit.
@@ -83,58 +80,45 @@ static int calculate_box_addr(int box, size_t len, u8 quirks)
* the packed value ends within pbuf. Must be smaller than, or equal
* to, startbit.
* @pbuflen: The length in bytes of the packed buffer pointed to by @pbuf.
- * @op: If PACK, then uval will be treated as const pointer and copied (packed)
- * into pbuf, between startbit and endbit.
- * If UNPACK, then pbuf will be treated as const pointer and the logical
- * value between startbit and endbit will be copied (unpacked) to uval.
* @quirks: A bit mask of QUIRK_LITTLE_ENDIAN, QUIRK_LSW32_IS_FIRST and
* QUIRK_MSB_ON_THE_RIGHT.
*
- * Note: this is deprecated, prefer to use pack() or unpack() in new code.
- *
* Return: 0 on success, EINVAL or ERANGE if called incorrectly. Assuming
- * correct usage, return code may be discarded.
- * If op is PACK, pbuf is modified.
- * If op is UNPACK, uval is modified.
+ * correct usage, return code may be discarded. The @pbuf memory will
+ * be modified on success.
*/
-int packing(void *pbuf, u64 *uval, int startbit, int endbit, size_t pbuflen,
- enum packing_op op, u8 quirks)
+int pack(void *pbuf, u64 uval, size_t startbit, size_t endbit, size_t pbuflen,
+ u8 quirks)
{
- /* Number of bits for storing "uval"
- * also width of the field to access in the pbuf
- */
- u64 value_width;
/* Logical byte indices corresponding to the
* start and end of the field.
*/
int plogical_first_u8, plogical_last_u8, box;
+ /* width of the field to access in the pbuf */
+ u64 value_width;
/* startbit is expected to be larger than endbit, and both are
* expected to be within the logically addressable range of the buffer.
*/
- if (unlikely(startbit < endbit || startbit >= 8 * pbuflen || endbit < 0))
+ if (unlikely(startbit < endbit || startbit >= 8 * pbuflen))
/* Invalid function call */
return -EINVAL;
value_width = startbit - endbit + 1;
- if (value_width > 64)
+ if (unlikely(value_width > 64))
return -ERANGE;
/* Check if "uval" fits in "value_width" bits.
* If value_width is 64, the check will fail, but any
* 64-bit uval will surely fit.
*/
- if (op == PACK && value_width < 64 && (*uval >= (1ull << value_width)))
+ if (unlikely(value_width < 64 && uval >= (1ull << value_width)))
/* Cannot store "uval" inside "value_width" bits.
* Truncating "uval" is most certainly not desirable,
* so simply erroring out is appropriate.
*/
return -ERANGE;
- /* Initialize parameter */
- if (op == UNPACK)
- *uval = 0;
-
/* Iterate through an idealistic view of the pbuf as an u64 with
* no quirks, u8 by u8 (aligned at u8 boundaries), from high to low
* logical bit significance. "box" denotes the current logical u8.
@@ -144,11 +128,12 @@ int packing(void *pbuf, u64 *uval, int startbit, int endbit, size_t pbuflen,
for (box = plogical_first_u8; box >= plogical_last_u8; box--) {
/* Bit indices into the currently accessed 8-bit box */
- int box_start_bit, box_end_bit, box_addr;
+ size_t box_start_bit, box_end_bit, box_addr;
u8 box_mask;
/* Corresponding bits from the unpacked u64 parameter */
- int proj_start_bit, proj_end_bit;
+ size_t proj_start_bit, proj_end_bit;
u64 proj_mask;
+ u64 pval;
/* This u8 may need to be accessed in its entirety
* (from bit 7 to bit 0), or not, depending on the
@@ -182,66 +167,21 @@ int packing(void *pbuf, u64 *uval, int startbit, int endbit, size_t pbuflen,
*/
box_addr = calculate_box_addr(box, pbuflen, quirks);
- if (op == UNPACK) {
- u64 pval;
+ /* Write to pbuf, read from uval */
+ pval = uval & proj_mask;
+ pval >>= proj_end_bit;
+ if (quirks & QUIRK_MSB_ON_THE_RIGHT)
+ adjust_for_msb_right_quirk(&pval,
+ &box_start_bit,
+ &box_end_bit,
+ &box_mask);
- /* Read from pbuf, write to uval */
- pval = ((u8 *)pbuf)[box_addr] & box_mask;
- if (quirks & QUIRK_MSB_ON_THE_RIGHT)
- adjust_for_msb_right_quirk(&pval,
- &box_start_bit,
- &box_end_bit,
- &box_mask);
-
- pval >>= box_end_bit;
- pval <<= proj_end_bit;
- *uval &= ~proj_mask;
- *uval |= pval;
- } else {
- u64 pval;
-
- /* Write to pbuf, read from uval */
- pval = (*uval) & proj_mask;
- pval >>= proj_end_bit;
- if (quirks & QUIRK_MSB_ON_THE_RIGHT)
- adjust_for_msb_right_quirk(&pval,
- &box_start_bit,
- &box_end_bit,
- &box_mask);
-
- pval <<= box_end_bit;
- ((u8 *)pbuf)[box_addr] &= ~box_mask;
- ((u8 *)pbuf)[box_addr] |= pval;
- }
+ pval <<= box_end_bit;
+ ((u8 *)pbuf)[box_addr] &= ~box_mask;
+ ((u8 *)pbuf)[box_addr] |= pval;
}
return 0;
}
-EXPORT_SYMBOL(packing);
-
-/**
- * pack - Pack u64 number into bitfield of buffer.
- *
- * @pbuf: Pointer to a buffer holding the packed value.
- * @uval: Pointer to an u64 holding the unpacked value.
- * @startbit: The index (in logical notation, compensated for quirks) where
- * the packed value starts within pbuf. Must be larger than, or
- * equal to, endbit.
- * @endbit: The index (in logical notation, compensated for quirks) where
- * the packed value ends within pbuf. Must be smaller than, or equal
- * to, startbit.
- * @pbuflen: The length in bytes of the packed buffer pointed to by @pbuf.
- * @quirks: A bit mask of QUIRK_LITTLE_ENDIAN, QUIRK_LSW32_IS_FIRST and
- * QUIRK_MSB_ON_THE_RIGHT.
- *
- * Return: 0 on success, EINVAL or ERANGE if called incorrectly. Assuming
- * correct usage, return code may be discarded. The @pbuf memory will
- * be modified on success.
- */
-int pack(void *pbuf, const u64 *uval, size_t startbit, size_t endbit,
- size_t pbuflen, u8 quirks)
-{
- return packing(pbuf, (u64 *)uval, startbit, endbit, pbuflen, PACK, quirks);
-}
EXPORT_SYMBOL(pack);
/**
@@ -266,8 +206,129 @@ EXPORT_SYMBOL(pack);
int unpack(const void *pbuf, u64 *uval, size_t startbit, size_t endbit,
size_t pbuflen, u8 quirks)
{
- return packing((void *)pbuf, uval, startbit, endbit, pbuflen, UNPACK, quirks);
+ /* Logical byte indices corresponding to the
+ * start and end of the field.
+ */
+ int plogical_first_u8, plogical_last_u8, box;
+ /* width of the field to access in the pbuf */
+ u64 value_width;
+
+ /* startbit is expected to be larger than endbit, and both are
+ * expected to be within the logically addressable range of the buffer.
+ */
+ if (unlikely(startbit < endbit || startbit >= 8 * pbuflen))
+ /* Invalid function call */
+ return -EINVAL;
+
+ value_width = startbit - endbit + 1;
+ if (unlikely(value_width > 64))
+ return -ERANGE;
+
+ /* Initialize parameter */
+ *uval = 0;
+
+ /* Iterate through an idealistic view of the pbuf as an u64 with
+ * no quirks, u8 by u8 (aligned at u8 boundaries), from high to low
+ * logical bit significance. "box" denotes the current logical u8.
+ */
+ plogical_first_u8 = startbit / 8;
+ plogical_last_u8 = endbit / 8;
+
+ for (box = plogical_first_u8; box >= plogical_last_u8; box--) {
+ /* Bit indices into the currently accessed 8-bit box */
+ size_t box_start_bit, box_end_bit, box_addr;
+ u8 box_mask;
+ /* Corresponding bits from the unpacked u64 parameter */
+ size_t proj_start_bit, proj_end_bit;
+ u64 proj_mask;
+ u64 pval;
+
+ /* This u8 may need to be accessed in its entirety
+ * (from bit 7 to bit 0), or not, depending on the
+ * input arguments startbit and endbit.
+ */
+ if (box == plogical_first_u8)
+ box_start_bit = startbit % 8;
+ else
+ box_start_bit = 7;
+ if (box == plogical_last_u8)
+ box_end_bit = endbit % 8;
+ else
+ box_end_bit = 0;
+
+ /* We have determined the box bit start and end.
+ * Now we calculate where this (masked) u8 box would fit
+ * in the unpacked (CPU-readable) u64 - the u8 box's
+ * projection onto the unpacked u64. Though the
+ * box is u8, the projection is u64 because it may fall
+ * anywhere within the unpacked u64.
+ */
+ proj_start_bit = ((box * 8) + box_start_bit) - endbit;
+ proj_end_bit = ((box * 8) + box_end_bit) - endbit;
+ proj_mask = GENMASK_ULL(proj_start_bit, proj_end_bit);
+ box_mask = GENMASK_ULL(box_start_bit, box_end_bit);
+
+ /* Determine the offset of the u8 box inside the pbuf,
+ * adjusted for quirks. The adjusted box_addr will be used for
+ * effective addressing inside the pbuf (so it's not
+ * logical any longer).
+ */
+ box_addr = calculate_box_addr(box, pbuflen, quirks);
+
+ /* Read from pbuf, write to uval */
+ pval = ((u8 *)pbuf)[box_addr] & box_mask;
+ if (quirks & QUIRK_MSB_ON_THE_RIGHT)
+ adjust_for_msb_right_quirk(&pval,
+ &box_start_bit,
+ &box_end_bit,
+ &box_mask);
+
+ pval >>= box_end_bit;
+ pval <<= proj_end_bit;
+ *uval &= ~proj_mask;
+ *uval |= pval;
+ }
+ return 0;
}
EXPORT_SYMBOL(unpack);
+/**
+ * packing - Convert numbers (currently u64) between a packed and an unpacked
+ * format. Unpacked means laid out in memory in the CPU's native
+ * understanding of integers, while packed means anything else that
+ * requires translation.
+ *
+ * @pbuf: Pointer to a buffer holding the packed value.
+ * @uval: Pointer to an u64 holding the unpacked value.
+ * @startbit: The index (in logical notation, compensated for quirks) where
+ * the packed value starts within pbuf. Must be larger than, or
+ * equal to, endbit.
+ * @endbit: The index (in logical notation, compensated for quirks) where
+ * the packed value ends within pbuf. Must be smaller than, or equal
+ * to, startbit.
+ * @pbuflen: The length in bytes of the packed buffer pointed to by @pbuf.
+ * @op: If PACK, then uval will be treated as const pointer and copied (packed)
+ * into pbuf, between startbit and endbit.
+ * If UNPACK, then pbuf will be treated as const pointer and the logical
+ * value between startbit and endbit will be copied (unpacked) to uval.
+ * @quirks: A bit mask of QUIRK_LITTLE_ENDIAN, QUIRK_LSW32_IS_FIRST and
+ * QUIRK_MSB_ON_THE_RIGHT.
+ *
+ * Note: this is deprecated, prefer to use pack() or unpack() in new code.
+ *
+ * Return: 0 on success, EINVAL or ERANGE if called incorrectly. Assuming
+ * correct usage, return code may be discarded.
+ * If op is PACK, pbuf is modified.
+ * If op is UNPACK, uval is modified.
+ */
+int packing(void *pbuf, u64 *uval, int startbit, int endbit, size_t pbuflen,
+ enum packing_op op, u8 quirks)
+{
+ if (op == PACK)
+ return pack(pbuf, *uval, startbit, endbit, pbuflen, quirks);
+
+ return unpack(pbuf, uval, startbit, endbit, pbuflen, quirks);
+}
+EXPORT_SYMBOL(packing);
+
MODULE_DESCRIPTION("Generic bitfield packing and unpacking");
--
2.46.2.828.g9e56e24342b6
Powered by blists - more mailing lists