linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [bug report] xfs: scrub inodes
Date: Mon, 6 Nov 2017 11:38:19 -0800	[thread overview]
Message-ID: <20171106193819.GB6244@magnolia> (raw)
In-Reply-To: <20171104080243.b5gchkmrofmkdbdf@mwanda>

On Sat, Nov 04, 2017 at 11:02:43AM +0300, Dan Carpenter wrote:
> Hello Darrick J. Wong,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 80e4e1268802: "xfs: scrub inodes" from Oct 17, 2017, leads 
> to the following Smatch complaint:
> 
> fs/xfs/scrub/inode.c:356 xfs_scrub_dinode()
> 	 error: we previously assumed 'sc->ip' could be null (see line 338)
> 
> fs/xfs/scrub/inode.c
>    337	
>    338			if (dip->di_mode == 0 && sc->ip)
>                                                  ^^^^^^
> The patch adds a check if sc->ip is NULL
> 
>    339				xfs_scrub_ino_set_corrupt(sc, ino, bp);
>    340	
>    341			if (dip->di_projid_hi != 0 &&
>    342			    !xfs_sb_version_hasprojid32bit(&mp->m_sb))
>    343				xfs_scrub_ino_set_corrupt(sc, ino, bp);
>    344			break;
>    345		default:
>    346			xfs_scrub_ino_set_corrupt(sc, ino, bp);
>    347			return;
>    348		}
>    349	
>    350		/*
>    351		 * di_uid/di_gid -- -1 isn't invalid, but there's no way that
>    352		 * userspace could have created that.
>    353		 */
>    354		if (dip->di_uid == cpu_to_be32(-1U) ||
>    355		    dip->di_gid == cpu_to_be32(-1U))
>    356			xfs_scrub_ino_set_warning(sc, bp);
>                                                   ^^
> But later we pass it to xfs_scrub_ino_set_warning() and it gets
> dereferenced without checking...  I don't know the rules about sc->ip
> well enough to say when it's NULL or not...

xfs_scrub_ino_set_warning and xfs_scrub_ino_set_preen both need to
take the inode number as a parameter and not rely on sc->ip pointing
anywhere.  I'll send a fix shortly; thank you for bringing this to my
attention.

--D

>    357	
>    358		/* di_format */
> 
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

      reply	other threads:[~2017-11-06 19:38 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-04  8:02 [bug report] xfs: scrub inodes Dan Carpenter
2017-11-06 19:38 ` Darrick J. Wong [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171106193819.GB6244@magnolia \
    --to=darrick.wong@oracle.com \
    --cc=dan.carpenter@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).