Re: [PATCH] freevxfs: reject invalid OLT record sizes
From: Christoph Hellwig
Date: Wed Jul 22 2026 - 08:10:45 EST
On Wed, Jul 22, 2026 at 07:57:06PM +0900, Sangho Lee wrote:
> vxfs_read_olt() walks image-controlled Object Location Table records by
> adding each record's on-disk olt_size to the current cursor:
>
> oaddr += fs32_to_cpu(infp, ocp->olt_size);
>
> The value is not checked before it is used. A crafted VxFS image can set a
> record size to zero, which prevents the cursor from advancing and leaves
> mount(2) spinning in the kernel. Oversized values can also move the cursor
> past the mapped OLT block without first rejecting the malformed image.
>
> Reject malformed OLT header and record sizes before using them. A valid
> header must place the first record after the header and within the mapped
> OLT extent. Each record must be at least large enough to contain the common
> record header and must fit in the remaining extent.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Sangho Lee <kudo3228@xxxxxxxxx>
> ---
> fs/freevxfs/vxfs_olt.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/fs/freevxfs/vxfs_olt.c b/fs/freevxfs/vxfs_olt.c
> index 23f35187c289..e5fb3b86d88e 100644
> --- a/fs/freevxfs/vxfs_olt.c
> +++ b/fs/freevxfs/vxfs_olt.c
> @@ -56,6 +56,7 @@
> struct buffer_head *bp;
> struct vxfs_olt *op;
> char *oaddr, *eaddr;
> + u32 olt_size;
>
> bp = sb_bread(sbp, vxfs_oblock(sbp, infp->vsi_oltext, bsize));
> if (!bp || !bp->b_data)
> @@ -77,12 +78,21 @@
> goto fail;
> }
>
> - oaddr = bp->b_data + fs32_to_cpu(infp, op->olt_size);
> eaddr = bp->b_data + (infp->vsi_oltsize * sbp->s_blocksize);
> + olt_size = fs32_to_cpu(infp, op->olt_size);
> + if (olt_size < sizeof(*op) || olt_size > eaddr - bp->b_data) {
> + pr_notice("vxfs: invalid olt header size\n");
> + goto fail;
> + }
> + oaddr = bp->b_data + olt_size;
>
> while (oaddr < eaddr) {
> struct vxfs_oltcommon *ocp =
> (struct vxfs_oltcommon *)oaddr;
> + u32 rec_size = fs32_to_cpu(infp, ocp->olt_size);
Overly long line.
Otherwise this does looks sane, but given that freevxfs had
exactly one user in the last 20 years, what is the point?