Hi Andrew,
On 20/10/2023 21:26, Andrew Cooper wrote:
When in use, the spew:
(XEN) Assertion '!memchr_inv(b->ptr.buffer + MIN_BLOCK_SIZE, POISON_BYTE,
(b->size & BLOCK_SIZE_MASK) - MIN_BLOCK_SIZE)' failed at common/xmalloc_tlsf.c:246
is unweidly and meaningless to non-Xen developers. Therefore:
* Switch to IS_ENABLED(). There's no need for full #ifdef-ary.
* Pull memchr_inv() out into the if(), and provide a better error message.
Signed-off-by: Andrew Cooper <[email protected]>
---
CC: George Dunlap <[email protected]>
CC: Jan Beulich <[email protected]>
CC: Stefano Stabellini <[email protected]>
CC: Wei Liu <[email protected]>
CC: Julien Grall <[email protected]>
CC: Roger Pau Monné <[email protected]>
Observations from the debugging of:
https://github.com/Dasharo/dasharo-issues/issues/488
There's a further bug. XMEM_POOL_POISON can be enabled in release builds,
where the ASSERT() gets dropped.
However, upping to a BUG() can't provide any helpful message out to the user.
I tried modifying BUG() to take an optional message, but xen/bug.h needs
untangling substantially before that will work, and I don't have time right now.
Do we actually care about the registers values? If not, we can either use:
printk(...);
BUG();
Or
panic(...);
This would allow us to use XMEM_POOL_POISON in prod build before BUG()
can accept string.
---
xen/common/xmalloc_tlsf.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
index 349b31cb4cc1..13305cd87c2f 100644
--- a/xen/common/xmalloc_tlsf.c
+++ b/xen/common/xmalloc_tlsf.c
@@ -249,11 +249,11 @@ static inline void EXTRACT_BLOCK(struct bhdr *b, struct
xmem_pool *p, int fl,
}
b->ptr.free_ptr = (struct free_ptr) {NULL, NULL};
-#ifdef CONFIG_XMEM_POOL_POISON
- if ( (b->size & BLOCK_SIZE_MASK) > MIN_BLOCK_SIZE )
- ASSERT(!memchr_inv(b->ptr.buffer + MIN_BLOCK_SIZE, POISON_BYTE,
- (b->size & BLOCK_SIZE_MASK) - MIN_BLOCK_SIZE));
-#endif /* CONFIG_XMEM_POOL_POISON */
+ if ( IS_ENABLED(CONFIG_XMEM_POOL_POISON) &&
+ (b->size & BLOCK_SIZE_MASK) > MIN_BLOCK_SIZE &&
+ memchr_inv(b->ptr.buffer + MIN_BLOCK_SIZE, POISON_BYTE,
+ (b->size & BLOCK_SIZE_MASK) - MIN_BLOCK_SIZE) )
+ ASSERT(!"XMEM Pool corruption found");
}
/**
@@ -261,11 +261,10 @@ static inline void EXTRACT_BLOCK(struct bhdr *b, struct
xmem_pool *p, int fl,
*/
static inline void INSERT_BLOCK(struct bhdr *b, struct xmem_pool *p, int fl,
int sl)
{
-#ifdef CONFIG_XMEM_POOL_POISON
- if ( (b->size & BLOCK_SIZE_MASK) > MIN_BLOCK_SIZE )
+ if ( IS_ENABLED(CONFIG_XMEM_POOL_POISON) &&
+ (b->size & BLOCK_SIZE_MASK) > MIN_BLOCK_SIZE )
memset(b->ptr.buffer + MIN_BLOCK_SIZE, POISON_BYTE,
(b->size & BLOCK_SIZE_MASK) - MIN_BLOCK_SIZE);
-#endif /* CONFIG_XMEM_POOL_POISON */
b->ptr.free_ptr = (struct free_ptr) {NULL, p->matrix[fl][sl]};
if ( p->matrix[fl][sl] )
Cheers,
--
Julien Grall
