Re: [PATCH 2/5] xfs: check healthmon outbuffer space correctly

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Aug 25, 2026 at 08:44:51AM +0200, Christoph Hellwig wrote:
> On Mon, Aug 24, 2026 at 10:36:28PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM notices that the outbuf space check in xfs_healthmon_format_pop
> > isn't quite correct -- it checks that there's enough space to write a
> > xfs_healthmon_event object, but the outbuffer is supposed to contain
> > xfs_health_monitor_event objects.  Fix this by adding a helper, and
> > refactoring all three outbuf size checks to use it.
> > 
> > Cc: <stable@vger.kernel.org> # v7.0
> > Fixes: b3a289a2a9397b ("xfs: create event queuing, formatting, and discovery infrastructure")
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > Assisted-by: LOLLM # finding obvious bugs
> > ---
> >  fs/xfs/xfs_healthmon.c |   16 +++++++++++++---
> >  1 file changed, 13 insertions(+), 3 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/xfs_healthmon.c b/fs/xfs/xfs_healthmon.c
> > index 166ef0d5864486..a2ae15a262a510 100644
> > --- a/fs/xfs/xfs_healthmon.c
> > +++ b/fs/xfs/xfs_healthmon.c
> > @@ -739,6 +739,12 @@ static const unsigned int type_map[] = {
> >  	[XFS_HEALTHMON_DATALOST]	= XFS_HEALTH_MONITOR_TYPE_DATALOST,
> >  };
> >  
> > +static inline bool
> > +xfs_healthmon_check_outbuffer_space(const struct xfs_healthmon *hm)
> > +{
> > +	return hm->bufhead + sizeof(struct xfs_health_monitor_event) <= hm->bufsize;
> 
> Overly long line.

Will fix.

> >  		memcpy(hm->buffer + hm->bufhead, &hme, sizeof(hme));
> >  		hm->bufhead += sizeof(hme);
> >  	}
> > @@ -891,7 +897,11 @@ xfs_healthmon_format_pop(
> >  {
> >  	struct xfs_healthmon_event *event;
> >  
> > -	if (hm->bufhead + sizeof(*event) > hm->bufsize)
> > +	/*
> > +	 * Don't bother if there's not enough space to format even one event in
> > +	 * the outbuffer.
> > +	 */
> > +	if (!xfs_healthmon_check_outbuffer_space(hm))
> >  		return NULL;
> >  
> >  	mutex_lock(&hm->lock);
> 
> This is a bit annoying as we now require the type name instead of an
> object, and the v0 implies there could be other formats.  But I guess
> for now it is fine..

<nod> v0 is the only format, eventually that helper would end up looking
like this:

static inline bool
xfs_healthmon_check_outbuffer_space(const struct xfs_healthmon *hm)
{
	size_t	event_sz = 0;

	switch (hm->format) {
	case XFS_HEALTHMON_V0:
		event_sz = sizeof(struct xfs_health_monitor_event);
		break;
	case XFS_HEALTHMON_V1:
		event_sz = sizeof(struct xfs_health_monitor_event_v1);
		break;
	default:
		ASSERT(0);
		break;
	}

	return hm->bufhead + event_sz <= hm->bufsize;
}

--D