From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Price <anprice@redhat.com>
Date: Thu, 24 Aug 2017 13:23:31 +0100
Subject: [Cluster-devel] [bug report] GFS2: Withdraw for IO errors
 writing to the journal or statfs
In-Reply-To: <20170824110357.b3t7z2wcnl7doeyt@mwanda>
References: <20170824110357.b3t7z2wcnl7doeyt@mwanda>
Message-ID: <20170824122330.GA16546@cicero>
List-Id: <cluster-devel.redhat.com>
To: cluster-devel.redhat.com
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

On Thu, Aug 24, 2017 at 02:03:57PM +0300, Dan Carpenter wrote:
> Hello Bob Peterson,
> 
> The patch 9bfef7554e20: "GFS2: Withdraw for IO errors writing to the
> journal or statfs" from Aug 16, 2017, leads to the following static
> checker warning:
> 
> 	fs/gfs2/super.c:949 gfs2_sync_fs()
> 	error: we previously assumed 'sdp' could be null (see line 947)
> 
> fs/gfs2/super.c
>    942  static int gfs2_sync_fs(struct super_block *sb, int wait)
>    943  {
>    944          struct gfs2_sbd *sdp = sb->s_fs_info;
>    945  
>    946          gfs2_quota_sync(sb, -1);
>    947          if (wait && sdp)
>                             ^^^
> Existing code checks for NULL.

The NULL check seemed odd to me, and other ->sync_fs implementations don't check it, but digging through the history there's a reason it was added (see 9171f5a ). That said, I can't see the quota_off code path that it was guarding against any more, so perhaps it is no longer required.

Andy

> 
>    948                  gfs2_log_flush(sdp, NULL, NORMAL_FLUSH);
>    949          return sdp->sd_log_error;
>                        ^^^^^^^^^^^^^^^^^
> Patch adds unchecked dereference.