* [bug report] xfs: use perag for ialloc btree cursors
@ 2021-06-10 6:16 Dan Carpenter
2021-06-10 22:37 ` [PATCH] xfs: perag may be null in xfs_imap() Dave Chinner
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2021-06-10 6:16 UTC (permalink / raw)
To: dchinner; +Cc: linux-xfs
Hello Dave Chinner,
This is a semi-automatic email about new static checker warnings.
The patch 7b13c5155182: "xfs: use perag for ialloc btree cursors"
from Jun 2, 2021, leads to the following Smatch complaint:
fs/xfs/libxfs/xfs_ialloc.c:2403 xfs_imap()
error: we previously assumed 'pag' could be null (see line 2294)
fs/xfs/libxfs/xfs_ialloc.c
2286 ASSERT(ino != NULLFSINO);
2287
2288 /*
2289 * Split up the inode number into its parts.
2290 */
2291 pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, ino));
2292 agino = XFS_INO_TO_AGINO(mp, ino);
2293 agbno = XFS_AGINO_TO_AGBNO(mp, agino);
2294 if (!pag || agbno >= mp->m_sb.sb_agblocks ||
^^^
Checks for NULL
2295 ino != XFS_AGINO_TO_INO(mp, pag->pag_agno, agino)) {
2296 error = -EINVAL;
2297 #ifdef DEBUG
2298 /*
2299 * Don't output diagnostic information for untrusted inodes
2300 * as they can be invalid without implying corruption.
2301 */
2302 if (flags & XFS_IGET_UNTRUSTED)
2303 goto out_drop;
^^^^^^^^^^^^^
2304 if (!pag) {
^^^^
2305 xfs_alert(mp,
2306 "%s: agno (%d) >= mp->m_sb.sb_agcount (%d)",
2307 __func__, XFS_INO_TO_AGNO(mp, ino),
2308 mp->m_sb.sb_agcount);
2309 }
2310 if (agbno >= mp->m_sb.sb_agblocks) {
2311 xfs_alert(mp,
2312 "%s: agbno (0x%llx) >= mp->m_sb.sb_agblocks (0x%lx)",
2313 __func__, (unsigned long long)agbno,
2314 (unsigned long)mp->m_sb.sb_agblocks);
2315 }
2316 if (pag && ino != XFS_AGINO_TO_INO(mp, pag->pag_agno, agino)) {
2317 xfs_alert(mp,
2318 "%s: ino (0x%llx) != XFS_AGINO_TO_INO() (0x%llx)",
2319 __func__, ino,
2320 XFS_AGINO_TO_INO(mp, pag->pag_agno, agino));
2321 }
2322 xfs_stack_trace();
2323 #endif /* DEBUG */
2324 goto out_drop;
2325 }
2326
2327 /*
2328 * For bulkstat and handle lookups, we have an untrusted inode number
2329 * that we have to verify is valid. We cannot do this just by reading
2330 * the inode buffer as it may have been unlinked and removed leaving
2331 * inodes in stale state on disk. Hence we have to do a btree lookup
2332 * in all cases where an untrusted inode number is passed.
2333 */
2334 if (flags & XFS_IGET_UNTRUSTED) {
2335 error = xfs_imap_lookup(mp, tp, pag, agino, agbno,
2336 &chunk_agbno, &offset_agbno, flags);
2337 if (error)
2338 goto out_drop;
2339 goto out_map;
2340 }
2341
2342 /*
2343 * If the inode cluster size is the same as the blocksize or
2344 * smaller we get to the buffer by simple arithmetics.
2345 */
2346 if (M_IGEO(mp)->blocks_per_cluster == 1) {
2347 offset = XFS_INO_TO_OFFSET(mp, ino);
2348 ASSERT(offset < mp->m_sb.sb_inopblock);
2349
2350 imap->im_blkno = XFS_AGB_TO_DADDR(mp, pag->pag_agno, agbno);
2351 imap->im_len = XFS_FSB_TO_BB(mp, 1);
2352 imap->im_boffset = (unsigned short)(offset <<
2353 mp->m_sb.sb_inodelog);
2354 error = 0;
2355 goto out_drop;
2356 }
2357
2358 /*
2359 * If the inode chunks are aligned then use simple maths to
2360 * find the location. Otherwise we have to do a btree
2361 * lookup to find the location.
2362 */
2363 if (M_IGEO(mp)->inoalign_mask) {
2364 offset_agbno = agbno & M_IGEO(mp)->inoalign_mask;
2365 chunk_agbno = agbno - offset_agbno;
2366 } else {
2367 error = xfs_imap_lookup(mp, tp, pag, agino, agbno,
2368 &chunk_agbno, &offset_agbno, flags);
2369 if (error)
2370 goto out_drop;
2371 }
2372
2373 out_map:
2374 ASSERT(agbno >= chunk_agbno);
2375 cluster_agbno = chunk_agbno +
2376 ((offset_agbno / M_IGEO(mp)->blocks_per_cluster) *
2377 M_IGEO(mp)->blocks_per_cluster);
2378 offset = ((agbno - cluster_agbno) * mp->m_sb.sb_inopblock) +
2379 XFS_INO_TO_OFFSET(mp, ino);
2380
2381 imap->im_blkno = XFS_AGB_TO_DADDR(mp, pag->pag_agno, cluster_agbno);
2382 imap->im_len = XFS_FSB_TO_BB(mp, M_IGEO(mp)->blocks_per_cluster);
2383 imap->im_boffset = (unsigned short)(offset << mp->m_sb.sb_inodelog);
2384
2385 /*
2386 * If the inode number maps to a block outside the bounds
2387 * of the file system then return NULL rather than calling
2388 * read_buf and panicing when we get an error from the
2389 * driver.
2390 */
2391 if ((imap->im_blkno + imap->im_len) >
2392 XFS_FSB_TO_BB(mp, mp->m_sb.sb_dblocks)) {
2393 xfs_alert(mp,
2394 "%s: (im_blkno (0x%llx) + im_len (0x%llx)) > sb_dblocks (0x%llx)",
2395 __func__, (unsigned long long) imap->im_blkno,
2396 (unsigned long long) imap->im_len,
2397 XFS_FSB_TO_BB(mp, mp->m_sb.sb_dblocks));
2398 error = -EINVAL;
2399 goto out_drop;
2400 }
2401 error = 0;
2402 out_drop:
2403 xfs_perag_put(pag);
^^^
Dereferenced inside the function call
2404 return error;
2405 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] xfs: perag may be null in xfs_imap()
2021-06-10 6:16 [bug report] xfs: use perag for ialloc btree cursors Dan Carpenter
@ 2021-06-10 22:37 ` Dave Chinner
2021-06-10 22:48 ` Darrick J. Wong
2021-06-11 0:57 ` Allison Henderson
0 siblings, 2 replies; 4+ messages in thread
From: Dave Chinner @ 2021-06-10 22:37 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-xfs, djwong
From: Dave Chinner <dchinner@redhat.com>
Dan Carpenter's static checker reported:
The patch 7b13c5155182: "xfs: use perag for ialloc btree cursors"
from Jun 2, 2021, leads to the following Smatch complaint:
fs/xfs/libxfs/xfs_ialloc.c:2403 xfs_imap()
error: we previously assumed 'pag' could be null (see line 2294)
And it's right. Fix it.
Fixes: 7b13c5155182 ("xfs: use perag for ialloc btree cursors")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/libxfs/xfs_ialloc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 654a8d9681e1..57d9cb632983 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -2398,7 +2398,8 @@ xfs_imap(
}
error = 0;
out_drop:
- xfs_perag_put(pag);
+ if (pag)
+ xfs_perag_put(pag);
return error;
}
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: perag may be null in xfs_imap()
2021-06-10 22:37 ` [PATCH] xfs: perag may be null in xfs_imap() Dave Chinner
@ 2021-06-10 22:48 ` Darrick J. Wong
2021-06-11 0:57 ` Allison Henderson
1 sibling, 0 replies; 4+ messages in thread
From: Darrick J. Wong @ 2021-06-10 22:48 UTC (permalink / raw)
To: Dave Chinner; +Cc: Dan Carpenter, linux-xfs
On Fri, Jun 11, 2021 at 08:37:47AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Dan Carpenter's static checker reported:
>
> The patch 7b13c5155182: "xfs: use perag for ialloc btree cursors"
> from Jun 2, 2021, leads to the following Smatch complaint:
>
> fs/xfs/libxfs/xfs_ialloc.c:2403 xfs_imap()
> error: we previously assumed 'pag' could be null (see line 2294)
>
> And it's right. Fix it.
>
> Fixes: 7b13c5155182 ("xfs: use perag for ialloc btree cursors")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
Looks correct to me,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> fs/xfs/libxfs/xfs_ialloc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 654a8d9681e1..57d9cb632983 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -2398,7 +2398,8 @@ xfs_imap(
> }
> error = 0;
> out_drop:
> - xfs_perag_put(pag);
> + if (pag)
> + xfs_perag_put(pag);
> return error;
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: perag may be null in xfs_imap()
2021-06-10 22:37 ` [PATCH] xfs: perag may be null in xfs_imap() Dave Chinner
2021-06-10 22:48 ` Darrick J. Wong
@ 2021-06-11 0:57 ` Allison Henderson
1 sibling, 0 replies; 4+ messages in thread
From: Allison Henderson @ 2021-06-11 0:57 UTC (permalink / raw)
To: Dave Chinner, Dan Carpenter; +Cc: linux-xfs, djwong
On 6/10/21 3:37 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Dan Carpenter's static checker reported:
>
> The patch 7b13c5155182: "xfs: use perag for ialloc btree cursors"
> from Jun 2, 2021, leads to the following Smatch complaint:
>
> fs/xfs/libxfs/xfs_ialloc.c:2403 xfs_imap()
> error: we previously assumed 'pag' could be null (see line 2294)
>
> And it's right. Fix it.
>
> Fixes: 7b13c5155182 ("xfs: use perag for ialloc btree cursors")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
Looks ok
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
> ---
> fs/xfs/libxfs/xfs_ialloc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 654a8d9681e1..57d9cb632983 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -2398,7 +2398,8 @@ xfs_imap(
> }
> error = 0;
> out_drop:
> - xfs_perag_put(pag);
> + if (pag)
> + xfs_perag_put(pag);
> return error;
> }
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-11 0:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-10 6:16 [bug report] xfs: use perag for ialloc btree cursors Dan Carpenter
2021-06-10 22:37 ` [PATCH] xfs: perag may be null in xfs_imap() Dave Chinner
2021-06-10 22:48 ` Darrick J. Wong
2021-06-11 0:57 ` Allison Henderson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.