tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

lib: packing: demote truncation error in pack() to a warning in __pack()

Most of the sanity checks in pack() and unpack() can be covered at
compile time. There is only one exception, and that is truncation of the
uval during a pack() operation.

We'd like the error-less __pack() to catch that condition as well. But
at the same time, it is currently the responsibility of consumer drivers
(currently just sja1105) to print anything at all when this error
occurs, and then discard the return code.

We can just print a loud warning in the library code and continue with
the truncated __pack() operation. In practice, having the warning is
very important, see commit 24deec6b9e4a ("net: dsa: sja1105: disallow
C45 transactions on the BASE-TX MDIO bus") where the bug was caught
exactly by noticing this print.

Add the first print to the packing library, and at the same time remove
the print for the same condition from the sja1105 driver, to avoid
double printing.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Link: https://patch.msgid.link/20241210-packing-pack-fields-and-ice-implementation-v10-2-ee56a47479ac@intel.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

authored by

Vladimir Oltean and committed by
Jakub Kicinski 48c27527 c4117091

+12 -22
+2 -6
drivers/net/dsa/sja1105/sja1105_static_config.c
··· 26 26 pr_err("Start bit (%d) expected to be larger than end (%d)\n", 27 27 start, end); 28 28 } else if (rc == -ERANGE) { 29 - if ((start - end + 1) > 64) 30 - pr_err("Field %d-%d too large for 64 bits!\n", 31 - start, end); 32 - else 33 - pr_err("Cannot store %llx inside bits %d-%d (would truncate)\n", 34 - *val, start, end); 29 + pr_err("Field %d-%d too large for 64 bits!\n", 30 + start, end); 35 31 } 36 32 dump_stack(); 37 33 }
+10 -16
lib/packing.c
··· 59 59 */ 60 60 int plogical_first_u8 = startbit / BITS_PER_BYTE; 61 61 int plogical_last_u8 = endbit / BITS_PER_BYTE; 62 + int value_width = startbit - endbit + 1; 62 63 int box; 64 + 65 + /* Check if "uval" fits in "value_width" bits. 66 + * The test only works for value_width < 64, but in the latter case, 67 + * any 64-bit uval will surely fit. 68 + */ 69 + WARN(value_width < 64 && uval >= (1ull << value_width), 70 + "Cannot store 0x%llx inside bits %zu-%zu - will truncate\n", 71 + uval, startbit, endbit); 63 72 64 73 /* Iterate through an idealistic view of the pbuf as an u64 with 65 74 * no quirks, u8 by u8 (aligned at u8 boundaries), from high to low ··· 152 143 int pack(void *pbuf, u64 uval, size_t startbit, size_t endbit, size_t pbuflen, 153 144 u8 quirks) 154 145 { 155 - /* width of the field to access in the pbuf */ 156 - u64 value_width; 157 - 158 146 /* startbit is expected to be larger than endbit, and both are 159 147 * expected to be within the logically addressable range of the buffer. 160 148 */ ··· 159 153 /* Invalid function call */ 160 154 return -EINVAL; 161 155 162 - value_width = startbit - endbit + 1; 163 - if (unlikely(value_width > 64)) 164 - return -ERANGE; 165 - 166 - /* Check if "uval" fits in "value_width" bits. 167 - * If value_width is 64, the check will fail, but any 168 - * 64-bit uval will surely fit. 169 - */ 170 - if (value_width < 64 && uval >= (1ull << value_width)) 171 - /* Cannot store "uval" inside "value_width" bits. 172 - * Truncating "uval" is most certainly not desirable, 173 - * so simply erroring out is appropriate. 174 - */ 156 + if (unlikely(startbit - endbit >= 64)) 175 157 return -ERANGE; 176 158 177 159 __pack(pbuf, uval, startbit, endbit, pbuflen, quirks);