> -----Original Message-----
> From: Jan Beulich <[email protected]>
> Sent: 05 July 2019 14:41
> To: Paul Durrant <[email protected]>; [email protected]
> Cc: Julien Grall <[email protected]>; Andrew Cooper
> <[email protected]>; George Dunlap
> <[email protected]>; Ian Jackson <[email protected]>; Stefano
> Stabellini
> <[email protected]>; Konrad Rzeszutek Wilk <[email protected]>; Tim
> (Xen.org) <[email protected]>;
> Wei Liu <[email protected]>
> Subject: Re: [PATCH v2 2/2] xmalloc: add a Kconfig option to poison free pool
> memory
>
> On 05.07.2019 11:02, Paul Durrant wrote:
> > --- a/xen/Kconfig.debug
> > +++ b/xen/Kconfig.debug
> > @@ -105,6 +105,13 @@ config DEBUG_TRACE
> > either directly to the console or are printed to console in case of
> > a system crash.
> >
> > +config XMEM_POOL_POISON
> > + bool "Poison free xenpool blocks"
> > + default DEBUG
> > + ---help---
> > + Poison free blocks with 0xAA bytes and verify them when a block is
> > + allocated in order to spot use-after-free issues.
>
> There looks to be a spaces vs tabs problem here: Only the help
> text has tabs for initial indentation.
Oh yes. I'll fix that.
>
> > --- a/xen/common/xmalloc_tlsf.c
> > +++ b/xen/common/xmalloc_tlsf.c
> > @@ -238,6 +238,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, 0xAA,
> > + (b->size & BLOCK_SIZE_MASK) - MIN_BLOCK_SIZE));
> > +#endif /* CONFIG_XMEM_POOL_POISON */
> > }
> >
> > /**
> > @@ -245,6 +250,11 @@ 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 )
> > + memset(b->ptr.buffer + MIN_BLOCK_SIZE, 0xAA,
> > + (b->size & BLOCK_SIZE_MASK) - MIN_BLOCK_SIZE);
> > +#endif /* CONFIG_XMEM_POOL_POISON */
>
> Can you tie together the two instances of 0xAA via a #define,
> please?
>
Ok, sure.
> It would also be nice if both #ifdef blocks were separated from
> their neighboring code by a blank line, despite the file using
> blank lines rather sparingly so far.
>
> With the adjustments here I think it would be best if you could
> also resend patch 1 with the slightly adjusted commit message.
Alright, will do.
Paul
>
> Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
