[PATCH] csm: Sanitise alignment constraint in Legacy16GetTableAddress
The alignment constraint is defined in the CSM specifications as "Bit mapped. First non-zero bit from the right is the alignment." Use __fls() to sanitise the alignment given that definition, since passing a non-power-of-two alignment to _malloc() isn't going to work well. And cope with being passed zero, which was happening for the E820 table allocation from EDK2. Signed-off-by: David Woodhouse <dwmw2@infradead.org> --- src/fw/csm.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/fw/csm.c b/src/fw/csm.c index 03b4bb8..bf7b8f5 100644 --- a/src/fw/csm.c +++ b/src/fw/csm.c @@ -264,6 +264,13 @@ handle_csm_0006(struct bregs *regs) dprintf(3, "Legacy16GetTableAddress size %x align %x region %d\n", size, align, region); + // DX = Required address alignment. Bit mapped. + // First non-zero bit from the right is the alignment.*/ + if (align) + align = 1 << __ffs(align); + else + align = 1; + if (region & 2) chunk = _malloc(&ZoneLow, size, align); if (!chunk && (region & 1))
On Wed, Jun 12, 2019 at 05:56:23PM +0100, David Woodhouse wrote:
The alignment constraint is defined in the CSM specifications as "Bit mapped. First non-zero bit from the right is the alignment."
Use __fls() to sanitise the alignment given that definition, since passing a non-power-of-two alignment to _malloc() isn't going to work well. And cope with being passed zero, which was happening for the E820 table allocation from EDK2.
Signed-off-by: David Woodhouse <dwmw2@infradead.org> --- src/fw/csm.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/src/fw/csm.c b/src/fw/csm.c index 03b4bb8..bf7b8f5 100644 --- a/src/fw/csm.c +++ b/src/fw/csm.c @@ -264,6 +264,13 @@ handle_csm_0006(struct bregs *regs) dprintf(3, "Legacy16GetTableAddress size %x align %x region %d\n", size, align, region);
+ // DX = Required address alignment. Bit mapped. + // First non-zero bit from the right is the alignment.*/ + if (align) + align = 1 << __ffs(align); + else + align = 1; +
Thanks. This should also enforce "align >= MALLOC_MIN_ALIGN". -Kevin
The alignment constraint is defined in the CSM specifications as "Bit mapped. First non-zero bit from the right is the alignment." Use __fls() to sanitise the alignment given that definition, since passing a non-power-of-two alignment to _malloc() isn't going to work well. And cope with being passed zero, which was happening for the E820 table allocation from EDK2. Signed-off-by: David Woodhouse <dwmw2@infradead.org> --- v2: Enforce MALLOC_MIN_ALIGN src/fw/csm.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/fw/csm.c b/src/fw/csm.c index 03b4bb8..3fcc252 100644 --- a/src/fw/csm.c +++ b/src/fw/csm.c @@ -258,11 +258,21 @@ handle_csm_0006(struct bregs *regs) u16 region = regs->bx; // (1 for F000 seg, 2 for E000 seg, 0 for either) void *chunk = NULL; + dprintf(3, "Legacy16GetTableAddress size %x align %x region %d\n", + size, align, region); + if (!region) region = 3; - dprintf(3, "Legacy16GetTableAddress size %x align %x region %d\n", - size, align, region); + // DX = Required address alignment. Bit mapped. + // First non-zero bit from the right is the alignment.*/ + if (align) { + align = 1 << __ffs(align); + if (align < MALLOC_MIN_ALIGN) + align = MALLOC_MIN_ALIGN; + } else { + align = MALLOC_MIN_ALIGN; + } if (region & 2) chunk = _malloc(&ZoneLow, size, align);
On Thu, Jun 13, 2019 at 04:25:13PM +0100, David Woodhouse wrote:
The alignment constraint is defined in the CSM specifications as "Bit mapped. First non-zero bit from the right is the alignment."
Use __fls() to sanitise the alignment given that definition, since passing a non-power-of-two alignment to _malloc() isn't going to work well. And cope with being passed zero, which was happening for the E820 table allocation from EDK2.
Thanks. I committed this change. -Kevin
participants (2)
-
David Woodhouse -
Kevin O'Connor