Hi Faiz, > -----Original Message----- > From: Faiz Abbas <faiz_ab...@ti.com> > Sent: Wednesday, September 4, 2019 2:44 PM > To: Alexey Brodkin <abrod...@synopsys.com> > Cc: paule...@forallsecure.com; tr...@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_ab...@ti.com> > >> Sent: Wednesday, September 4, 2019 12:22 PM > >> To: u-boot@lists.denx.de > >> Cc: paule...@forallsecure.com; faiz_ab...@ti.com; Alexey Brodkin > >> <abrod...@synopsys.com>; > >> tr...@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. -Alexey _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
