xfs
[Top] [All Lists]

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

To: xfs@xxxxxxxxxxx
Subject: Re: [PATCH] xfs: validate quota log items during log recovery
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 16 Mar 2009 03:54:07 -0400
In-reply-to: <20090303175427.GA20582@xxxxxxxxxxxxx>
References: <20090303175427.GA20582@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
ping?

On Tue, Mar 03, 2009 at 12:54:27PM -0500, Christoph Hellwig wrote:
> Arkadiusz has been seeing really strange crashes in xfs_qm_dqcheck that
> I can only explain by a log item beeing too smal to actually fit the
> 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>
> 
> Index: xfs/fs/xfs/xfs_log_recover.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_log_recover.c 2009-03-02 04:15:11.410430892 +0100
> +++ xfs/fs/xfs/xfs_log_recover.c      2009-03-02 04:16:29.649444226 +0100
> @@ -1975,16 +1975,26 @@ 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 ||
> +                         item->ri_buf[i].i_len < sizeof(xfs_dqblk_t)) {
> +                             cmn_err(CE_ALERT,
> +     "XFS: dquot too small (%d) in xlog_recover_do_reg_buffer.",
> +                                             item->ri_buf[i].i_len);
> +                             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 +2625,15 @@ 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 ||
> +         item->ri_buf[1].i_len < sizeof(xfs_dqblk_t)) {
> +             cmn_err(CE_ALERT,
> +     "XFS: dquot too small (%d) in xlog_recover_do_dquot_trans.",
> +                     item->ri_buf[1].i_len);
> +             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
---end quoted text---

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