On 2/19/26 06:24, Aswin Murugan wrote:
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
Yeah, but it's important to ensure size == cell->size for normal bitfields. We
don't want to allow e.g.
supplying a 4-byte cell for a mac address. We probably don't want to allow
storing e.g. 0xdeadbeef in a
cell of size 2 either...
--Sean
