220 lines
6.4 KiB
Diff
220 lines
6.4 KiB
Diff
From 342b52596ce27f30b59622b67da188c4a3adbeee Mon Sep 17 00:00:00 2001
|
|
From: Wu Guanghao <wuguanghao3@huawei.com>
|
|
Date: Wed, 21 Jun 2023 09:25:27 +0000
|
|
Subject: [PATCH] mkfs.xfs: fix segmentation fault caused by accessing a null
|
|
pointer
|
|
|
|
We encountered a segfault while testing the mkfs.xfs + iscsi.
|
|
|
|
(gdb) bt
|
|
#0 libxfs_log_sb (tp=0xaaaafaea0630) at xfs_sb.c:810
|
|
#1 0x0000aaaaca991468 in __xfs_trans_commit (tp=<optimized out>, tp@entry=0xaaaafaea0630, regrant=regrant@entry=true) at trans.c:995
|
|
#2 0x0000aaaaca991790 in libxfs_trans_roll (tpp=tpp@entry=0xfffffe1f3018) at trans.c:103
|
|
#3 0x0000aaaaca9bcde8 in xfs_dialloc_roll (agibp=0xaaaafaea2fa0, tpp=0xfffffe1f31c8) at xfs_ialloc.c:1561
|
|
#4 xfs_dialloc_try_ag (ok_alloc=true, new_ino=<synthetic pointer>, parent=0, pag=0xaaaafaea0210, tpp=0xfffffe1f31c8) at xfs_ialloc.c:1698
|
|
#5 xfs_dialloc (tpp=tpp@entry=0xfffffe1f31c8, parent=0, mode=mode@entry=16877, new_ino=new_ino@entry=0xfffffe1f3128) at xfs_ialloc.c:1776
|
|
#6 0x0000aaaaca9925b0 in libxfs_dir_ialloc (tpp=tpp@entry=0xfffffe1f31c8, dp=dp@entry=0x0, mode=mode@entry=16877, nlink=nlink@entry=1, rdev=rdev@entry=0, cr=cr@entry=0xfffffe1f31d0,
|
|
fsx=fsx@entry=0xfffffe1f36a4, ipp=ipp@entry=0xfffffe1f31c0) at util.c:525
|
|
#7 0x0000aaaaca988fac in parseproto (mp=0xfffffe1f36c8, pip=0x0, fsxp=0xfffffe1f36a4, pp=0xfffffe1f3370, name=0x0) at proto.c:552
|
|
#8 0x0000aaaaca9867a4 in main (argc=<optimized out>, argv=<optimized out>) at xfs_mkfs.c:4217
|
|
|
|
(gdb) p bp
|
|
$1 = 0x0
|
|
|
|
```
|
|
void
|
|
xfs_log_sb(
|
|
struct xfs_trans *tp)
|
|
{
|
|
// iscsi offline
|
|
...
|
|
// failed to read sb, bp = NULL
|
|
struct xfs_buf *bp = xfs_trans_getsb(tp);
|
|
...
|
|
}
|
|
```
|
|
|
|
When writing data to sb, if the device is abnormal at this time,
|
|
the bp may be empty. Using it without checking will result in
|
|
a segfault.
|
|
|
|
So before using it, we need to check if the bp is empty and return
|
|
the error.
|
|
|
|
Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com>
|
|
---
|
|
libxfs/trans.c | 4 +++-
|
|
libxfs/xfs_attr_leaf.c | 2 +-
|
|
libxfs/xfs_bmap.c | 8 ++++++--
|
|
libxfs/xfs_sb.c | 25 ++++++++++++++++++++++---
|
|
libxfs/xfs_sb.h | 2 +-
|
|
mkfs/proto.c | 15 ++++++++++++---
|
|
6 files changed, 45 insertions(+), 11 deletions(-)
|
|
|
|
diff --git a/libxfs/trans.c b/libxfs/trans.c
|
|
index fd2e6f9..a43472f 100644
|
|
--- a/libxfs/trans.c
|
|
+++ b/libxfs/trans.c
|
|
@@ -975,7 +975,9 @@ __xfs_trans_commit(
|
|
sbp->sb_fdblocks += tp->t_fdblocks_delta;
|
|
if (tp->t_frextents_delta)
|
|
sbp->sb_frextents += tp->t_frextents_delta;
|
|
- xfs_log_sb(tp);
|
|
+ error = xfs_log_sb(tp);
|
|
+ if (error)
|
|
+ goto out_unreserve;
|
|
}
|
|
|
|
trans_committed(tp);
|
|
diff --git a/libxfs/xfs_attr_leaf.c b/libxfs/xfs_attr_leaf.c
|
|
index cfb6bf1..9522155 100644
|
|
--- a/libxfs/xfs_attr_leaf.c
|
|
+++ b/libxfs/xfs_attr_leaf.c
|
|
@@ -629,7 +629,7 @@ xfs_sbversion_add_attr2(xfs_mount_t *mp, xfs_trans_t *tp)
|
|
if (!xfs_sb_version_hasattr2(&mp->m_sb)) {
|
|
xfs_sb_version_addattr2(&mp->m_sb);
|
|
spin_unlock(&mp->m_sb_lock);
|
|
- xfs_log_sb(tp);
|
|
+ ASSERT(!xfs_log_sb(tp));
|
|
} else
|
|
spin_unlock(&mp->m_sb_lock);
|
|
}
|
|
diff --git a/libxfs/xfs_bmap.c b/libxfs/xfs_bmap.c
|
|
index a548507..16dbe71 100644
|
|
--- a/libxfs/xfs_bmap.c
|
|
+++ b/libxfs/xfs_bmap.c
|
|
@@ -1122,8 +1122,12 @@ xfs_bmap_add_attrfork(
|
|
log_sb = true;
|
|
}
|
|
spin_unlock(&mp->m_sb_lock);
|
|
- if (log_sb)
|
|
- xfs_log_sb(tp);
|
|
+ if (log_sb) {
|
|
+ error = xfs_log_sb(tp);
|
|
+ if (error)
|
|
+ goto trans_cancel;
|
|
+ }
|
|
+
|
|
}
|
|
|
|
error = xfs_trans_commit(tp);
|
|
diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
|
|
index b2e214e..df7aec5 100644
|
|
--- a/libxfs/xfs_sb.c
|
|
+++ b/libxfs/xfs_sb.c
|
|
@@ -784,13 +784,16 @@ xfs_sb_mount_common(
|
|
* level of locking that is needed to protect the in-core superblock from
|
|
* concurrent access.
|
|
*/
|
|
-void
|
|
+int
|
|
xfs_log_sb(
|
|
struct xfs_trans *tp)
|
|
{
|
|
struct xfs_mount *mp = tp->t_mountp;
|
|
struct xfs_buf *bp = xfs_trans_getsb(tp);
|
|
|
|
+ if (!bp)
|
|
+ return -EIO;
|
|
+
|
|
/*
|
|
* Lazy sb counters don't update the in-core superblock so do that now.
|
|
* If this is at unmount, the counters will be exactly correct, but at
|
|
@@ -808,6 +811,8 @@ xfs_log_sb(
|
|
xfs_sb_to_disk(bp->b_addr, &mp->m_sb);
|
|
xfs_trans_buf_set_type(tp, bp, XFS_BLFT_SB_BUF);
|
|
xfs_trans_log_buf(tp, bp, 0, sizeof(struct xfs_dsb) - 1);
|
|
+
|
|
+ return 0;
|
|
}
|
|
|
|
/*
|
|
@@ -834,7 +839,13 @@ xfs_sync_sb(
|
|
if (error)
|
|
return error;
|
|
|
|
- xfs_log_sb(tp);
|
|
+ error = xfs_log_sb(tp);
|
|
+ if (error) {
|
|
+ xfs_trans_cancel(tp);
|
|
+ return error;
|
|
+ }
|
|
+
|
|
+
|
|
if (wait)
|
|
xfs_trans_set_sync(tp);
|
|
return xfs_trans_commit(tp);
|
|
@@ -931,7 +942,15 @@ xfs_sync_sb_buf(
|
|
return error;
|
|
|
|
bp = xfs_trans_getsb(tp);
|
|
- xfs_log_sb(tp);
|
|
+ if (!bp) {
|
|
+ xfs_trans_cancel(tp);
|
|
+ return -EIO;
|
|
+ }
|
|
+ error = xfs_log_sb(tp);
|
|
+ if (error) {
|
|
+ xfs_trans_cancel(tp);
|
|
+ return error;
|
|
+ }
|
|
xfs_trans_bhold(tp, bp);
|
|
xfs_trans_set_sync(tp);
|
|
error = xfs_trans_commit(tp);
|
|
diff --git a/libxfs/xfs_sb.h b/libxfs/xfs_sb.h
|
|
index 0c1602d..33ca6bb 100644
|
|
--- a/libxfs/xfs_sb.h
|
|
+++ b/libxfs/xfs_sb.h
|
|
@@ -13,7 +13,7 @@ struct xfs_trans;
|
|
struct xfs_fsop_geom;
|
|
struct xfs_perag;
|
|
|
|
-extern void xfs_log_sb(struct xfs_trans *tp);
|
|
+extern int xfs_log_sb(struct xfs_trans *tp);
|
|
extern int xfs_sync_sb(struct xfs_mount *mp, bool wait);
|
|
extern int xfs_sync_sb_buf(struct xfs_mount *mp);
|
|
extern void xfs_sb_mount_common(struct xfs_mount *mp, struct xfs_sb *sbp);
|
|
diff --git a/mkfs/proto.c b/mkfs/proto.c
|
|
index ef130ed..005b6e3 100644
|
|
--- a/mkfs/proto.c
|
|
+++ b/mkfs/proto.c
|
|
@@ -557,7 +557,10 @@ parseproto(
|
|
if (!pip) {
|
|
pip = ip;
|
|
mp->m_sb.sb_rootino = ip->i_ino;
|
|
- libxfs_log_sb(tp);
|
|
+ error = -libxfs_log_sb(tp);
|
|
+ if (error) {
|
|
+ fail(_("Log sb failed"), error);
|
|
+ }
|
|
isroot = 1;
|
|
} else {
|
|
libxfs_trans_ijoin(tp, pip, 0);
|
|
@@ -656,7 +659,10 @@ rtinit(
|
|
rbmip->i_diflags = XFS_DIFLAG_NEWRTBM;
|
|
*(uint64_t *)&VFS_I(rbmip)->i_atime = 0;
|
|
libxfs_trans_log_inode(tp, rbmip, XFS_ILOG_CORE);
|
|
- libxfs_log_sb(tp);
|
|
+ error = -libxfs_log_sb(tp);
|
|
+ if (error) {
|
|
+ fail(_("Log sb failed"), error);
|
|
+ }
|
|
mp->m_rbmip = rbmip;
|
|
error = -libxfs_dir_ialloc(&tp, NULL, S_IFREG, 1, 0,
|
|
&creds, &fsxattrs, &rsumip);
|
|
@@ -666,7 +672,10 @@ rtinit(
|
|
mp->m_sb.sb_rsumino = rsumip->i_ino;
|
|
rsumip->i_disk_size = mp->m_rsumsize;
|
|
libxfs_trans_log_inode(tp, rsumip, XFS_ILOG_CORE);
|
|
- libxfs_log_sb(tp);
|
|
+ error = -libxfs_log_sb(tp);
|
|
+ if (error) {
|
|
+ fail(_("Log sb failed"), error);
|
|
+ }
|
|
error = -libxfs_trans_commit(tp);
|
|
if (error)
|
|
fail(_("Completion of the realtime summary inode failed"),
|
|
--
|
|
2.33.0
|
|
|