Hi Sean,
On 2/18/2026 7:37 AM, Sean Anderson wrote:
On 2/17/26 14:05, Aswin Murugan wrote:
Hi Sean,
Thanks for your feedback.
To address stack smashing, which occurs when the cell size is larger
than 4 bytes, I will revert the `nvmem_cell_read()` call in
`reboot_mode_get()` to pass `sizeof(*mode)` as the size, consistent
with the original implementation.
Inside `nvmem_cell_read()`, will modify the validation check. Instead
of enforcing `size == cell->size`, will check if `cell->size` is
within the passed `size` (i.e., `cell->size <= size`) to correctly
support our 1-byte read case where the buffer is 4 bytes.
Then don't we need to zero out the rest of the buffer?
I would expect the check to be
if ((cell->bit_offset && size > cell->size) || size != cell->size)
return -EINVAL;
Also, i2c_eeprom_read() and misc_read() will continue to request a
read for cell->size instead of the passed buffer size, ensuring we
only read the valid data from the hardware.
Regarding the bit field logic, as you suggested will simplify the
bit‑field handling to max u32 size, which significantly reduces the
code complexity. Given this simplification, is it still necessary to
keep this bit‑manipulation logic behind a separate config option?
I think it's likely. You can check the growth with
scripts/bloat-o-meter in
Linux. Since this is mainly for U-Boot proper I'd say a couple hundred
bytes
is reasonable for this feature.
Kindly confirm if this approach aligns with your expectations.
Please remember to reply on the bottom :)
--Sean
The suggested check will fail for our case, we always pass size =
sizeof(u32) = 4 bytes regardless of the actual cell size.
Example: 1-byte cell with 7-bit field
cell->size = 1, bit_offset = 1, nbits = 7, size = 4
With suggested check: if ((cell->bit_offset && size > cell->size) ||
size != cell->size) -> (1 && 4 > 1) || 4 != 1 -> TRUE -> rejects valid case
If we have a check as below:
if (size < cell->size) -> 4 < 1 -> FALSE -> correctly allows
Then if cell has nbits set, bit field bounds can be validated separately
as follows:
bytes_needed = DIV_ROUND_UP(7+1, 8)
if (bytes_needed > cell->size || bytes_needed > sizeof(u32) || size !=
sizeof(u32))
This validates:
* bytes_needed > cell->size: Bit field doesn't exceed actual cell size
* bytes_needed > sizeof(u32): Bit field doesn't exceed maximum
supported size
* size != sizeof(u32): Buffer size is correct for bit field operations
Thanks
Aswin
