xfs
[Top] [All Lists]

Re: [PATCH] xfs: validate quota log items during log recovery

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH] xfs: validate quota log items during log recovery
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Wed, 27 May 2009 14:06:53 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20090527091734.GA21958@xxxxxxxxxxxxx>
References: <20090303175427.GA20582@xxxxxxxxxxxxx> <4A1C13DC.5000605@xxxxxxxxxxx> <20090527091734.GA21958@xxxxxxxxxxxxx>
User-agent: Thunderbird 2.0.0.21 (X11/20090320)
Christoph Hellwig wrote:

> Updated patch below:
> 
> 
> Subject: xfs: validate quota log items during log recovery
> From: Christoph Hellwig <hch@xxxxxx>
> 
> Arkadiusz has seen really strange crashes in xfs_qm_dqcheck that
> I can only explain by a log item being too smal to actually fit the
                                             ^^small ;)
> xfs_dqblk_t we're dereferencing all over xfs_qm_dqcheck.  So add
> graceful checks for NULL or too small quota items to the log recovery
> code.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

A bit more verbose now isn't it, but oh well :)

Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxxx>

> Index: xfs/fs/xfs/xfs_log_recover.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_log_recover.c 2009-05-27 10:40:03.752821404 +0200
> +++ xfs/fs/xfs/xfs_log_recover.c      2009-05-27 10:43:23.740939498 +0200
> @@ -1975,16 +1975,30 @@ xlog_recover_do_reg_buffer(
>               error = 0;
>               if (buf_f->blf_flags &
>                  (XFS_BLI_UDQUOT_BUF|XFS_BLI_PDQUOT_BUF|XFS_BLI_GDQUOT_BUF)) {
> +                     if (item->ri_buf[i].i_addr == NULL) {
> +                             cmn_err(CE_ALERT,
> +                                     "XFS: NULL dquot in %s.", __func__);
> +                             goto next;
> +                     }
> +                     if (item->ri_buf[i].i_len < sizeof(xfs_dqblk_t)) {
> +                             cmn_err(CE_ALERT,
> +                                     "XFS: dquot too small (%d) in %s.",
> +                                     item->ri_buf[i].i_len, __func__);
> +                             goto next;
> +                     }
>                       error = xfs_qm_dqcheck((xfs_disk_dquot_t *)
>                                              item->ri_buf[i].i_addr,
>                                              -1, 0, XFS_QMOPT_DOWARN,
>                                              "dquot_buf_recover");
> +                     if (error)
> +                             goto next;
>               }
> -             if (!error)
> -                     memcpy(xfs_buf_offset(bp,
> -                             (uint)bit << XFS_BLI_SHIFT),    /* dest */
> -                             item->ri_buf[i].i_addr,         /* source */
> -                             nbits<<XFS_BLI_SHIFT);          /* length */
> +
> +             memcpy(xfs_buf_offset(bp,
> +                     (uint)bit << XFS_BLI_SHIFT),    /* dest */
> +                     item->ri_buf[i].i_addr,         /* source */
> +                     nbits<<XFS_BLI_SHIFT);          /* length */
> + next:
>               i++;
>               bit += nbits;
>       }
> @@ -2615,7 +2629,19 @@ xlog_recover_do_dquot_trans(
>               return (0);
>  
>       recddq = (xfs_disk_dquot_t *)item->ri_buf[1].i_addr;
> -     ASSERT(recddq);
> +
> +     if (item->ri_buf[1].i_addr == NULL) {
> +             cmn_err(CE_ALERT,
> +                     "XFS: NULL dquot in %s.", __func__);
> +             return XFS_ERROR(EIO);
> +     }
> +     if (item->ri_buf[1].i_len < sizeof(xfs_dqblk_t)) {
> +             cmn_err(CE_ALERT,
> +                     "XFS: dquot too small (%d) in %s.",
> +                     item->ri_buf[1].i_len, __func__);
> +             return XFS_ERROR(EIO);
> +     }
> +
>       /*
>        * This type of quotas was turned off, so ignore this record.
>        */
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

<Prev in Thread] Current Thread [Next in Thread>