Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 25 Feb 2020 13:40:45 -0800
From:   Christoph Hellwig <hch@infradead.org>
To:     Qian Cai <cai@lca.pw>
Cc:     darrick.wong@oracle.com, hch@infradead.org,
        linux-xfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] xfs: fix an undefined behaviour in _da3_path_shift
Message-ID: <20200225214045.GA14399@infradead.org>
References: <1582660388-28735-1-git-send-email-cai@lca.pw>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1582660388-28735-1-git-send-email-cai@lca.pw>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Feb 25, 2020 at 02:53:08PM -0500, Qian Cai wrote:
> xfs_da3_path_shift() could see state->path.blk[-1] because
> state->path.active == 1 is a valid state when it tries to add an entry
> to a single dir leaf block and then to shift forward to see if
> there's a sibling block that would be a better place to put the new
> entry.

I think this needs a better explanation.  Something like:

In xfs_da3_path_shift() blk can be assigned to state->path.blk[-1] if
state->path.active is 1 (which is a valid state) when it tries to add an
entry > to a single dir leaf block and then to shift forward to see if
there's a sibling block that would be a better place to put the new
entry.  This causes a KASAN warning given negative array indices are
undefined behavior in C.  In practice the warning is entirely harmless
given that blk is never dereference in this case, but it is still better
to fix up the warning and slightly improve the code.