[PATCH v2 2/6] misc: Add support for bit fields in NVMEM cells

[Sean Anderson]

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