Hi Alexey,
On 04/09/19 5:16 PM, Alexey Brodkin wrote:
Hi Faiz,
-----Original Message----- From: Faiz Abbas faiz_abbas@ti.com Sent: Wednesday, September 4, 2019 2:44 PM To: Alexey Brodkin abrodkin@synopsys.com Cc: paulemge@forallsecure.com; trini@konsulko.com; u-boot@lists.denx.de Subject: Re: [PATCH] Revert "part: Allocate only one legacy_mbr buffer"
Hi Alexey,
On 04/09/19 3:42 PM, Alexey Brodkin wrote:
Hi Faiz,
-----Original Message----- From: Faiz Abbas faiz_abbas@ti.com Sent: Wednesday, September 4, 2019 12:22 PM To: u-boot@lists.denx.de Cc: paulemge@forallsecure.com; faiz_abbas@ti.com; Alexey Brodkin abrodkin@synopsys.com; trini@konsulko.com Subject: [PATCH] Revert "part: Allocate only one legacy_mbr buffer"
This reverts commit 8639e34d2c5e12cc2e45c95b1a2e97c22bf6a711.
The blk_dread() call following the allocation will read one block from the device. This will lead to overflow if the blocksize is greater than the size of legacy_mbr. Fix this by allocating one block size.
Did you read justification of my change that you're reverting now? With your change in place we'll allocate a buffer of size = (sizeof(legacy_mbr) * dev_desc->blksz).
Is that something you really want?
Oops. You're right. I thought your patch was changing buffer size from blksz to legacy_mbr.
I guess what you really want to do is to allocate buffer for "mbr" dynamically of size which is max(sizeof(legacy_mbr), dev_desc->blksz).
With the assumption that blksz is always greater than sizeof(legacy_mbr), this should work:
ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, DIV_ROUND_UP(dev_desc->blksz, sizeof(legacy_mbr)));
No this won't work. See ALLOC_CACHE_ALIGN_BUFFER does static allocation in compile-time while blksz is set in runtime.
That's why I mentioned switch to runtime allocation.
Hmm. So the problem isn't as much about allocating too much in the buffer but about using dynamically determined size for static array declaration. This is being used all over this disk/part_dos.c file. Shouldn't we fix all of that? Not sure how it has been working all along.
Thanks, Faiz