[PATCH] xfs_repair: validate inode di_flaaags field

[PATCH] xfs_repair: validate inode di_flaaags field

[Alex Elder]

(I may have missed it, but I don't believe Dave posted
this patch so I'm doing it for him to get it on the
list.  -Alex)

xfs-reapir is not validating the di_flags field in the inode for
sanity. Block fuzzing indicates that we are not picking situations
like the RT bit being set on filesystems without realtime devices.
    
Signed-off-by: Dave Chinner <dchinner@redhat.com>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_repair: validate inode di_flaaags field

[Alex Elder]

On Tue, 2011-02-22 at 14:43 -0600, Alex Elder wrote:
> (I may have missed it, but I don't believe Dave posted
> this patch so I'm doing it for him to get it on the
> list.  -Alex)
> 
> xfs-reapir is not validating the di_flags field in the inode for
> sanity. Block fuzzing indicates that we are not picking situations
> like the RT bit being set on filesystems without realtime devices.
>     
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> 

I guess I should have left it up to Dave...


 repair/dinode.c |   62
+++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/repair/dinode.c b/repair/dinode.c
index bf04c6e..fad15b2 100644
--- a/repair/dinode.c
+++ b/repair/dinode.c
@@ -2530,6 +2530,68 @@ process_dinode_int(xfs_mount_t *mp,
                goto clear_bad_out;
        }
 
+       /*
+        * check that we only have valid flags set, and those that are
set make
+        * sense.
+        */
+       if (dinoc->di_flags) {
+               uint16_t flags = be16_to_cpu(dinoc->di_flags);
+
+               if (flags & ~XFS_DIFLAG_ANY) {
+                       do_warn(_("Bad flags set in inode %llu"), lino);
+                       flags &= ~XFS_DIFLAG_ANY;
+               }
+
+               if (flags & (XFS_DIFLAG_REALTIME |
XFS_DIFLAG_RTINHERIT)) {
+                       /* need an rt-dev! */
+                       if (!rt_name) {
+                               do_warn(_(
+       "inode %llu has RT flag set but there is no RT device"), lino);
+                               flags &= ~(XFS_DIFLAG_REALTIME |
+                                               XFS_DIFLAG_RTINHERIT);
+                       }
+               }
+               if (flags & XFS_DIFLAG_NEWRTBM_BIT) {
+                       /* must be a rt bitmap inode */
+                       if (lino != mp->m_sb.sb_rbmino) {
+                               do_warn(_("inode %llu not rt bitmap"),
lino);
+                               flags &= ~XFS_DIFLAG_NEWRTBM_BIT;
+                       }
+               }
+               if (flags & (XFS_DIFLAG_RTINHERIT |
+                            XFS_DIFLAG_EXTSZINHERIT |
+                            XFS_DIFLAG_PROJINHERIT |
+                            XFS_DIFLAG_NOSYMLINKS)) {
+                       /* must be a directory */
+                       if (di_mode && !S_ISDIR(di_mode)) {
+                               do_warn(_(
+                       "directory flags set on non-directory inode %
llu"),
+                                       lino);
+                               flags &= ~(XFS_DIFLAG_RTINHERIT |
+                                               XFS_DIFLAG_EXTSZINHERIT
|
+                                               XFS_DIFLAG_PROJINHERIT |
+                                               XFS_DIFLAG_NOSYMLINKS);
+                       }
+               }
+               if (flags & (XFS_DIFLAG_REALTIME | XFS_XFLAG_EXTSIZE)) {
+                       /* must be a file */
+                       if (di_mode && !S_ISREG(di_mode)) {
+                               do_warn(_(
+                       "file flags set on non-file inode %llu"), lino);
+                               flags &= ~(XFS_DIFLAG_REALTIME |
+                                               XFS_XFLAG_EXTSIZE);
+                       }
+               }
+               if (!verify_mode && flags !=
be16_to_cpu(dinoc->di_flags)) {
+                       if (!no_modify) {
+                               do_warn(_(", fixing bad flags.\n"));
+                               dinoc->di_flags = cpu_to_be16(flags);
+                               *dirty = 1;
+                       } else
+                               do_warn(_(", would fix bad flags.\n"));
+               }
+       }
+
        if (verify_mode)
                return retval;

-- 
1.7.4


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_repair: validate inode di_flaaags field

[Alex Elder]

On Tue, 2011-02-22 at 14:45 -0600, Alex Elder wrote:
> On Tue, 2011-02-22 at 14:43 -0600, Alex Elder wrote:
> > (I may have missed it, but I don't believe Dave posted
> > this patch so I'm doing it for him to get it on the
> > list.  -Alex)
> > 
> > xfs-reapir is not validating the di_flags field in the inode for
> > sanity. Block fuzzing indicates that we are not picking situations
> > like the RT bit being set on filesystems without realtime devices.
> >     
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>

Turns out this was on the list, it just was sent as a
"will you please try this patch" message on the subject
of "XFS internal error xfs_iformat(realtime) even after
xfs_repair."


Anyway, now I've reviewed it, and it looks good.  The
typo's in the subject line and in the description ought
to be fixed before it's committed.

Reviewed-by: Alex Elder <aelder@sgi.com>



_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs