All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Jan Kara <jack@suse.cz>,
	Joseph Qi <joseph.qi@linux.alibaba.com>, Gang He <ghe@suse.com>,
	Mark Fasheh <mark@fasheh.com>, Joel Becker <jlbec@evilplan.org>,
	Junxiao Bi <junxiao.bi@oracle.com>,
	Changwei Ge <gechangwei@live.cn>, Jun Piao <piaojun@huawei.com>,
	"Markov, Andrey" <Markov.Andrey@Dell.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: [PATCH 5.4 27/58] ocfs2: fix data corruption after conversion from inline format
Date: Mon, 25 Oct 2021 21:14:44 +0200	[thread overview]
Message-ID: <20211025190941.984730604@linuxfoundation.org> (raw)
In-Reply-To: <20211025190937.555108060@linuxfoundation.org>

From: Jan Kara <jack@suse.cz>

commit 5314454ea3ff6fc746eaf71b9a7ceebed52888fa upstream.

Commit 6dbf7bb55598 ("fs: Don't invalidate page buffers in
block_write_full_page()") uncovered a latent bug in ocfs2 conversion
from inline inode format to a normal inode format.

The code in ocfs2_convert_inline_data_to_extents() attempts to zero out
the whole cluster allocated for file data by grabbing, zeroing, and
dirtying all pages covering this cluster.  However these pages are
beyond i_size, thus writeback code generally ignores these dirty pages
and no blocks were ever actually zeroed on the disk.

This oversight was fixed by commit 693c241a5f6a ("ocfs2: No need to zero
pages past i_size.") for standard ocfs2 write path, inline conversion
path was apparently forgotten; the commit log also has a reasoning why
the zeroing actually is not needed.

After commit 6dbf7bb55598, things became worse as writeback code stopped
invalidating buffers on pages beyond i_size and thus these pages end up
with clean PageDirty bit but with buffers attached to these pages being
still dirty.  So when a file is converted from inline format, then
writeback triggers, and then the file is grown so that these pages
become valid, the invalid dirtiness state is preserved,
mark_buffer_dirty() does nothing on these pages (buffers are already
dirty) but page is never written back because it is clean.  So data
written to these pages is lost once pages are reclaimed.

Simple reproducer for the problem is:

  xfs_io -f -c "pwrite 0 2000" -c "pwrite 2000 2000" -c "fsync" \
    -c "pwrite 4000 2000" ocfs2_file

After unmounting and mounting the fs again, you can observe that end of
'ocfs2_file' has lost its contents.

Fix the problem by not doing the pointless zeroing during conversion
from inline format similarly as in the standard write path.

[akpm@linux-foundation.org: fix whitespace, per Joseph]

Link: https://lkml.kernel.org/r/20210930095405.21433-1-jack@suse.cz
Fixes: 6dbf7bb55598 ("fs: Don't invalidate page buffers in block_write_full_page()")
Signed-off-by: Jan Kara <jack@suse.cz>
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Tested-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Acked-by: Gang He <ghe@suse.com>
Cc: Mark Fasheh <mark@fasheh.com>
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Junxiao Bi <junxiao.bi@oracle.com>
Cc: Changwei Ge <gechangwei@live.cn>
Cc: Jun Piao <piaojun@huawei.com>
Cc: "Markov, Andrey" <Markov.Andrey@Dell.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/ocfs2/alloc.c |   46 ++++++++++++----------------------------------
 1 file changed, 12 insertions(+), 34 deletions(-)

--- a/fs/ocfs2/alloc.c
+++ b/fs/ocfs2/alloc.c
@@ -7048,7 +7048,7 @@ void ocfs2_set_inode_data_inline(struct
 int ocfs2_convert_inline_data_to_extents(struct inode *inode,
 					 struct buffer_head *di_bh)
 {
-	int ret, i, has_data, num_pages = 0;
+	int ret, has_data, num_pages = 0;
 	int need_free = 0;
 	u32 bit_off, num;
 	handle_t *handle;
@@ -7057,26 +7057,17 @@ int ocfs2_convert_inline_data_to_extents
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	struct ocfs2_dinode *di = (struct ocfs2_dinode *)di_bh->b_data;
 	struct ocfs2_alloc_context *data_ac = NULL;
-	struct page **pages = NULL;
-	loff_t end = osb->s_clustersize;
+	struct page *page = NULL;
 	struct ocfs2_extent_tree et;
 	int did_quota = 0;
 
 	has_data = i_size_read(inode) ? 1 : 0;
 
 	if (has_data) {
-		pages = kcalloc(ocfs2_pages_per_cluster(osb->sb),
-				sizeof(struct page *), GFP_NOFS);
-		if (pages == NULL) {
-			ret = -ENOMEM;
-			mlog_errno(ret);
-			return ret;
-		}
-
 		ret = ocfs2_reserve_clusters(osb, 1, &data_ac);
 		if (ret) {
 			mlog_errno(ret);
-			goto free_pages;
+			goto out;
 		}
 	}
 
@@ -7096,7 +7087,8 @@ int ocfs2_convert_inline_data_to_extents
 	}
 
 	if (has_data) {
-		unsigned int page_end;
+		unsigned int page_end = min_t(unsigned, PAGE_SIZE,
+							osb->s_clustersize);
 		u64 phys;
 
 		ret = dquot_alloc_space_nodirty(inode,
@@ -7120,15 +7112,8 @@ int ocfs2_convert_inline_data_to_extents
 		 */
 		block = phys = ocfs2_clusters_to_blocks(inode->i_sb, bit_off);
 
-		/*
-		 * Non sparse file systems zero on extend, so no need
-		 * to do that now.
-		 */
-		if (!ocfs2_sparse_alloc(osb) &&
-		    PAGE_SIZE < osb->s_clustersize)
-			end = PAGE_SIZE;
-
-		ret = ocfs2_grab_eof_pages(inode, 0, end, pages, &num_pages);
+		ret = ocfs2_grab_eof_pages(inode, 0, page_end, &page,
+					   &num_pages);
 		if (ret) {
 			mlog_errno(ret);
 			need_free = 1;
@@ -7139,20 +7124,15 @@ int ocfs2_convert_inline_data_to_extents
 		 * This should populate the 1st page for us and mark
 		 * it up to date.
 		 */
-		ret = ocfs2_read_inline_data(inode, pages[0], di_bh);
+		ret = ocfs2_read_inline_data(inode, page, di_bh);
 		if (ret) {
 			mlog_errno(ret);
 			need_free = 1;
 			goto out_unlock;
 		}
 
-		page_end = PAGE_SIZE;
-		if (PAGE_SIZE > osb->s_clustersize)
-			page_end = osb->s_clustersize;
-
-		for (i = 0; i < num_pages; i++)
-			ocfs2_map_and_dirty_page(inode, handle, 0, page_end,
-						 pages[i], i > 0, &phys);
+		ocfs2_map_and_dirty_page(inode, handle, 0, page_end, page, 0,
+					 &phys);
 	}
 
 	spin_lock(&oi->ip_lock);
@@ -7183,8 +7163,8 @@ int ocfs2_convert_inline_data_to_extents
 	}
 
 out_unlock:
-	if (pages)
-		ocfs2_unlock_and_free_pages(pages, num_pages);
+	if (page)
+		ocfs2_unlock_and_free_pages(&page, num_pages);
 
 out_commit:
 	if (ret < 0 && did_quota)
@@ -7208,8 +7188,6 @@ out_commit:
 out:
 	if (data_ac)
 		ocfs2_free_alloc_context(data_ac);
-free_pages:
-	kfree(pages);
 	return ret;
 }
 



  parent reply	other threads:[~2021-10-25 19:34 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-25 19:14 [PATCH 5.4 00/58] 5.4.156-rc1 review Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 01/58] parisc: math-emu: Fix fall-through warnings Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 02/58] net: switchdev: do not propagate bridge updates across bridges Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 03/58] tee: optee: Fix missing devices unregister during optee_remove Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 04/58] ARM: dts: at91: sama5d2_som1_ek: disable ISC node by default Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 05/58] xtensa: xtfpga: use CONFIG_USE_OF instead of CONFIG_OF Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 06/58] xtensa: xtfpga: Try software restart before simulating CPU reset Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 07/58] NFSD: Keep existing listeners on portlist error Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 08/58] dma-debug: fix sg checks in debug_dma_map_sg() Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 09/58] ASoC: wm8960: Fix clock configuration on slave mode Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 10/58] netfilter: ipvs: make global sysctl readonly in non-init netns Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 11/58] lan78xx: select CRC32 Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 12/58] net: dsa: lantiq_gswip: fix register definition Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 13/58] NIOS2: irqflags: rename a redefined register name Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 14/58] net: hns3: reset DWRR of unused tc to zero Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 15/58] net: hns3: add limit ets dwrr bandwidth cannot be 0 Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 16/58] net: hns3: disable sriov before unload hclge layer Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 17/58] net: stmmac: Fix E2E delay mechanism Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 18/58] net: enetc: fix ethtool counter name for PM0_TERR Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 19/58] can: rcar_can: fix suspend/resume Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 20/58] can: peak_usb: pcan_usb_fd_decode_status(): fix back to ERROR_ACTIVE state notification Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 21/58] can: peak_pci: peak_pci_remove(): fix UAF Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 22/58] can: j1939: j1939_tp_rxtimer(): fix errant alert in j1939_tp_rxtimer Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 23/58] can: j1939: j1939_netdev_start(): fix UAF for rx_kref of j1939_priv Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 24/58] can: j1939: j1939_xtp_rx_dat_one(): cancel session if receive TP.DT with error length Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 25/58] can: j1939: j1939_xtp_rx_rts_session_new(): abort TP less than 9 bytes Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 26/58] ceph: fix handling of "meta" errors Greg Kroah-Hartman
2021-10-25 19:14 ` Greg Kroah-Hartman [this message]
2021-10-25 19:14 ` [PATCH 5.4 28/58] ocfs2: mount fails with buffer overflow in strlen Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 29/58] elfcore: correct reference to CONFIG_UML Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 30/58] vfs: check fd has read access in kernel_read_file_from_fd() Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 31/58] ALSA: usb-audio: Provide quirk for Sennheiser GSP670 Headset Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 32/58] ALSA: hda/realtek: Add quirk for Clevo PC50HS Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 33/58] ASoC: DAPM: Fix missing kctl change notifications Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 34/58] audit: fix possible null-pointer dereference in audit_filter_rules Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 35/58] powerpc64/idle: Fix SP offsets when saving GPRs Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 36/58] KVM: PPC: Book3S HV: Fix stack handling in idle_kvm_start_guest() Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 37/58] KVM: PPC: Book3S HV: Make idle_kvm_start_guest() return 0 if it went to guest Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 38/58] powerpc/idle: Dont corrupt back chain when going idle Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 39/58] mm, slub: fix mismatch between reconstructed freelist depth and cnt Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 40/58] mm, slub: fix potential memoryleak in kmem_cache_open() Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 41/58] nfc: nci: fix the UAF of rf_conn_info object Greg Kroah-Hartman
2021-10-25 19:14 ` [PATCH 5.4 42/58] isdn: cpai: check ctr->cnr to avoid array index out of bound Greg Kroah-Hartman
2021-10-25 19:15 ` [PATCH 5.4 43/58] netfilter: Kconfig: use default y instead of m for bool config option Greg Kroah-Hartman
2021-10-25 19:15 ` [PATCH 5.4 44/58] selftests: netfilter: remove stray bash debug line Greg Kroah-Hartman
2021-10-25 19:15 ` [PATCH 5.4 45/58] gcc-plugins/structleak: add makefile var for disabling structleak Greg Kroah-Hartman
2021-10-25 19:15 ` [PATCH 5.4 46/58] btrfs: deal with errors when checking if a dir entry exists during log replay Greg Kroah-Hartman
2021-10-25 19:15 ` [PATCH 5.4 47/58] net: stmmac: add support for dwmac 3.40a Greg Kroah-Hartman
2021-10-25 19:15 ` [PATCH 5.4 48/58] ARM: dts: spear3xx: Fix gmac node Greg Kroah-Hartman
2021-10-25 19:15 ` [PATCH 5.4 49/58] isdn: mISDN: Fix sleeping function called from invalid context Greg Kroah-Hartman
2021-10-25 19:15 ` [PATCH 5.4 50/58] platform/x86: intel_scu_ipc: Update timeout value in comment Greg Kroah-Hartman
2021-10-25 19:15 ` [PATCH 5.4 51/58] ALSA: hda: avoid write to STATESTS if controller is in reset Greg Kroah-Hartman
2021-10-25 19:15 ` [PATCH 5.4 52/58] Input: snvs_pwrkey - add clk handling Greg Kroah-Hartman
2021-10-25 19:15 ` [PATCH 5.4 53/58] scsi: core: Fix shost->cmd_per_lun calculation in scsi_add_host_with_dma() Greg Kroah-Hartman
2021-10-25 19:15 ` [PATCH 5.4 54/58] usbnet: sanity check for maxpacket Greg Kroah-Hartman
2021-10-25 19:15 ` [PATCH 5.4 55/58] net: mdiobus: Fix memory leak in __mdiobus_register Greg Kroah-Hartman
2021-10-25 19:15 ` [PATCH 5.4 56/58] tracing: Have all levels of checks prevent recursion Greg Kroah-Hartman
2021-10-25 19:15 ` [PATCH 5.4 57/58] ARM: 9122/1: select HAVE_FUTEX_CMPXCHG Greg Kroah-Hartman
2021-10-25 19:15 ` [PATCH 5.4 58/58] pinctrl: stm32: use valid pin identifier in stm32_pinctrl_resume() Greg Kroah-Hartman
2021-10-25 19:55 ` [PATCH 5.4 00/58] 5.4.156-rc1 review Florian Fainelli
2021-10-26  8:29 ` Naresh Kamboju
2021-10-26  9:16 ` Jon Hunter
2021-10-26 17:26 ` Shuah Khan
2021-10-26 18:21 ` Sudip Mukherjee
2021-10-26 19:16 ` Guenter Roeck

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=20211025190941.984730604@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=Markov.Andrey@Dell.com \
    --cc=akpm@linux-foundation.org \
    --cc=gechangwei@live.cn \
    --cc=ghe@suse.com \
    --cc=jack@suse.cz \
    --cc=jlbec@evilplan.org \
    --cc=joseph.qi@linux.alibaba.com \
    --cc=junxiao.bi@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark@fasheh.com \
    --cc=piaojun@huawei.com \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.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 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.