linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: gregkh@linuxfoundation.org
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Casey Schaufler <casey@schaufler-ca.com>,
	Filipe Manana <fdmanana@suse.com>,
	David Sterba <dsterba@suse.com>
Subject: [PATCH 5.11 19/44] btrfs: fix warning when creating a directory with smack enabled
Date: Mon,  8 Mar 2021 13:34:57 +0100	[thread overview]
Message-ID: <20210308122719.527723820@linuxfoundation.org> (raw)
In-Reply-To: <20210308122718.586629218@linuxfoundation.org>

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

From: Filipe Manana <fdmanana@suse.com>

commit fd57a98d6f0c98fa295813087f13afb26c224e73 upstream.

When we have smack enabled, during the creation of a directory smack may
attempt to add a "smack transmute" xattr on the inode, which results in
the following warning and trace:

  WARNING: CPU: 3 PID: 2548 at fs/btrfs/transaction.c:537 start_transaction+0x489/0x4f0
  Modules linked in: nft_objref nf_conntrack_netbios_ns (...)
  CPU: 3 PID: 2548 Comm: mkdir Not tainted 5.9.0-rc2smack+ #81
  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-2.fc32 04/01/2014
  RIP: 0010:start_transaction+0x489/0x4f0
  Code: e9 be fc ff ff (...)
  RSP: 0018:ffffc90001887d10 EFLAGS: 00010202
  RAX: ffff88816f1e0000 RBX: 0000000000000201 RCX: 0000000000000003
  RDX: 0000000000000201 RSI: 0000000000000002 RDI: ffff888177849000
  RBP: ffff888177849000 R08: 0000000000000001 R09: 0000000000000004
  R10: ffffffff825e8f7a R11: 0000000000000003 R12: ffffffffffffffe2
  R13: 0000000000000000 R14: ffff88803d884270 R15: ffff8881680d8000
  FS:  00007f67317b8440(0000) GS:ffff88817bcc0000(0000) knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: 00007f67247a22a8 CR3: 000000004bfbc002 CR4: 0000000000370ee0
  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
  DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
  Call Trace:
   ? slab_free_freelist_hook+0xea/0x1b0
   ? trace_hardirqs_on+0x1c/0xe0
   btrfs_setxattr_trans+0x3c/0xf0
   __vfs_setxattr+0x63/0x80
   smack_d_instantiate+0x2d3/0x360
   security_d_instantiate+0x29/0x40
   d_instantiate_new+0x38/0x90
   btrfs_mkdir+0x1cf/0x1e0
   vfs_mkdir+0x14f/0x200
   do_mkdirat+0x6d/0x110
   do_syscall_64+0x2d/0x40
   entry_SYSCALL_64_after_hwframe+0x44/0xa9
  RIP: 0033:0x7f673196ae6b
  Code: 8b 05 11 (...)
  RSP: 002b:00007ffc3c679b18 EFLAGS: 00000246 ORIG_RAX: 0000000000000053
  RAX: ffffffffffffffda RBX: 00000000000001ff RCX: 00007f673196ae6b
  RDX: 0000000000000000 RSI: 00000000000001ff RDI: 00007ffc3c67a30d
  RBP: 00007ffc3c67a30d R08: 00000000000001ff R09: 0000000000000000
  R10: 000055d3e39fe930 R11: 0000000000000246 R12: 0000000000000000
  R13: 00007ffc3c679cd8 R14: 00007ffc3c67a30d R15: 00007ffc3c679ce0
  irq event stamp: 11029
  hardirqs last  enabled at (11037): [<ffffffff81153fe6>] console_unlock+0x486/0x670
  hardirqs last disabled at (11044): [<ffffffff81153c01>] console_unlock+0xa1/0x670
  softirqs last  enabled at (8864): [<ffffffff81e0102f>] asm_call_on_stack+0xf/0x20
  softirqs last disabled at (8851): [<ffffffff81e0102f>] asm_call_on_stack+0xf/0x20

This happens because at btrfs_mkdir() we call d_instantiate_new() while
holding a transaction handle, which results in the following call chain:

  btrfs_mkdir()
     trans = btrfs_start_transaction(root, 5);

     d_instantiate_new()
        smack_d_instantiate()
            __vfs_setxattr()
                btrfs_setxattr_trans()
                   btrfs_start_transaction()
                      start_transaction()
                         WARN_ON()
                           --> a tansaction start has TRANS_EXTWRITERS
                               set in its type
                         h->orig_rsv = h->block_rsv
                         h->block_rsv = NULL

     btrfs_end_transaction(trans)

Besides the warning triggered at start_transaction, we set the handle's
block_rsv to NULL which may cause some surprises later on.

So fix this by making btrfs_setxattr_trans() not start a transaction when
we already have a handle on one, stored in current->journal_info, and use
that handle. We are good to use the handle because at btrfs_mkdir() we did
reserve space for the xattr and the inode item.

Reported-by: Casey Schaufler <casey@schaufler-ca.com>
CC: stable@vger.kernel.org # 5.4+
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Tested-by: Casey Schaufler <casey@schaufler-ca.com>
Link: https://lore.kernel.org/linux-btrfs/434d856f-bd7b-4889-a6ec-e81aaebfa735@schaufler-ca.com/
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/btrfs/xattr.c |   31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

--- a/fs/btrfs/xattr.c
+++ b/fs/btrfs/xattr.c
@@ -229,11 +229,33 @@ int btrfs_setxattr_trans(struct inode *i
 {
 	struct btrfs_root *root = BTRFS_I(inode)->root;
 	struct btrfs_trans_handle *trans;
+	const bool start_trans = (current->journal_info == NULL);
 	int ret;
 
-	trans = btrfs_start_transaction(root, 2);
-	if (IS_ERR(trans))
-		return PTR_ERR(trans);
+	if (start_trans) {
+		/*
+		 * 1 unit for inserting/updating/deleting the xattr
+		 * 1 unit for the inode item update
+		 */
+		trans = btrfs_start_transaction(root, 2);
+		if (IS_ERR(trans))
+			return PTR_ERR(trans);
+	} else {
+		/*
+		 * This can happen when smack is enabled and a directory is being
+		 * created. It happens through d_instantiate_new(), which calls
+		 * smack_d_instantiate(), which in turn calls __vfs_setxattr() to
+		 * set the transmute xattr (XATTR_NAME_SMACKTRANSMUTE) on the
+		 * inode. We have already reserved space for the xattr and inode
+		 * update at btrfs_mkdir(), so just use the transaction handle.
+		 * We don't join or start a transaction, as that will reset the
+		 * block_rsv of the handle and trigger a warning for the start
+		 * case.
+		 */
+		ASSERT(strncmp(name, XATTR_SECURITY_PREFIX,
+			       XATTR_SECURITY_PREFIX_LEN) == 0);
+		trans = current->journal_info;
+	}
 
 	ret = btrfs_setxattr(trans, inode, name, value, size, flags);
 	if (ret)
@@ -244,7 +266,8 @@ int btrfs_setxattr_trans(struct inode *i
 	ret = btrfs_update_inode(trans, root, BTRFS_I(inode));
 	BUG_ON(ret);
 out:
-	btrfs_end_transaction(trans);
+	if (start_trans)
+		btrfs_end_transaction(trans);
 	return ret;
 }
 



  parent reply	other threads:[~2021-03-08 12:36 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-08 12:34 [PATCH 5.11 00/44] 5.11.5-rc1 review gregkh
2021-03-08 12:34 ` [PATCH 5.11 01/44] ALSA: hda/realtek: Enable headset mic of Acer SWIFT with ALC256 gregkh
2021-03-08 12:34 ` [PATCH 5.11 02/44] ALSA: usb-audio: use Corsair Virtuoso mapping for Corsair Virtuoso SE gregkh
2021-03-08 12:34 ` [PATCH 5.11 03/44] ALSA: usb-audio: Dont abort even if the clock rate differs gregkh
2021-03-08 12:34 ` [PATCH 5.11 04/44] ALSA: usb-audio: Drop bogus dB range in too low level gregkh
2021-03-08 12:34 ` [PATCH 5.11 05/44] ALSA: usb-audio: Allow modifying parameters with succeeding hw_params calls gregkh
2021-03-08 12:34 ` [PATCH 5.11 06/44] tpm, tpm_tis: Decorate tpm_tis_gen_interrupt() with request_locality() gregkh
2021-03-08 12:34 ` [PATCH 5.11 07/44] tpm, tpm_tis: Decorate tpm_get_timeouts() " gregkh
2021-03-08 12:34 ` [PATCH 5.11 08/44] btrfs: avoid double put of block group when emptying cluster gregkh
2021-03-08 12:34 ` [PATCH 5.11 09/44] btrfs: fix raid6 qstripe kmap gregkh
2021-03-08 12:34 ` [PATCH 5.11 10/44] btrfs: fix race between writes to swap files and scrub gregkh
2021-03-08 12:34 ` [PATCH 5.11 11/44] btrfs: fix race between swap file activation and snapshot creation gregkh
2021-03-08 12:34 ` [PATCH 5.11 12/44] btrfs: fix stale data exposure after cloning a hole with NO_HOLES enabled gregkh
2021-03-08 12:34 ` [PATCH 5.11 13/44] btrfs: tree-checker: do not error out if extent ref hash doesnt match gregkh
2021-03-08 12:34 ` [PATCH 5.11 14/44] btrfs: fix race between extent freeing/allocation when using bitmaps gregkh
2021-03-08 12:34 ` [PATCH 5.11 15/44] btrfs: validate qgroup inherit for SNAP_CREATE_V2 ioctl gregkh
2021-03-08 12:34 ` [PATCH 5.11 16/44] btrfs: free correct amount of space in btrfs_delayed_inode_reserve_metadata gregkh
2021-03-08 12:34 ` [PATCH 5.11 17/44] btrfs: fix spurious free_space_tree remount warning gregkh
2021-03-08 12:34 ` [PATCH 5.11 18/44] btrfs: unlock extents in btrfs_zero_range in case of quota reservation errors gregkh
2021-03-08 12:34 ` gregkh [this message]
2021-03-08 12:34 ` [PATCH 5.11 20/44] PM: runtime: Update device status before letting suppliers suspend gregkh
2021-03-08 12:34 ` [PATCH 5.11 21/44] ring-buffer: Force before_stamp and write_stamp to be different on discard gregkh
2021-03-08 12:35 ` [PATCH 5.11 22/44] io_uring: ignore double poll add on the same waitqueue head gregkh
2021-03-08 12:35 ` [PATCH 5.11 23/44] dm bufio: subtract the number of initial sectors in dm_bufio_get_device_size gregkh
2021-03-08 12:35 ` [PATCH 5.11 24/44] dm verity: fix FEC for RS roots unaligned to block size gregkh
2021-03-08 12:35 ` [PATCH 5.11 25/44] drm/amd/pm: correct Arcturus mmTHM_BACO_CNTL register address gregkh
2021-03-08 12:35 ` [PATCH 5.11 26/44] drm/amdgpu:disable VCN for Navi12 SKU gregkh
2021-03-08 12:35 ` [PATCH 5.11 27/44] drm/amdgpu: Only check for S0ix if AMD_PMC is configured gregkh
2021-03-08 12:35 ` [PATCH 5.11 28/44] drm/amdgpu: fix parameter error of RREG32_PCIE() in amdgpu_regs_pcie gregkh
2021-03-08 12:35 ` [PATCH 5.11 29/44] crypto - shash: reduce minimum alignment of shash_desc structure gregkh
2021-03-08 12:35 ` [PATCH 5.11 30/44] ALSA: ctxfi: cthw20k2: fix mask on conf to allow 4 bits gregkh
2021-03-08 12:35 ` [PATCH 5.11 31/44] ALSA: usb-audio: Fix Pioneer DJM devices URB_CONTROL request direction to set samplerate gregkh
2021-03-08 12:35 ` [PATCH 5.11 32/44] RDMA/cm: Fix IRQ restore in ib_send_cm_sidr_rep gregkh
2021-03-08 12:35 ` [PATCH 5.11 33/44] RDMA/rxe: Fix missing kconfig dependency on CRYPTO gregkh
2021-03-08 12:35 ` [PATCH 5.11 34/44] IB/mlx5: Add missing error code gregkh
2021-03-08 12:35 ` [PATCH 5.11 35/44] ALSA: hda: intel-nhlt: verify config type gregkh
2021-03-08 12:35 ` [PATCH 5.11 36/44] ftrace: Have recordmcount use w8 to read relp->r_info in arm64_is_fake_mcount gregkh
2021-03-08 12:35 ` [PATCH 5.11 37/44] ia64: dont call handle_signal() unless theres actually a signal queued gregkh
2021-03-08 12:35 ` [PATCH 5.11 38/44] rsxx: Return -EFAULT if copy_to_user() fails gregkh
2021-03-08 12:35 ` [PATCH 5.11 39/44] iommu/tegra-smmu: Fix mc errors on tegra124-nyan gregkh
2021-03-08 12:35 ` [PATCH 5.11 40/44] iommu: Dont use lazy flush for untrusted device gregkh
2021-03-08 12:35 ` [PATCH 5.11 41/44] iommu/vt-d: Fix status code for Allocate/Free PASID command gregkh
2021-03-08 12:35 ` [PATCH 5.11 42/44] btrfs: zoned: use sector_t for zone sectors gregkh
2021-03-08 12:35 ` [PATCH 5.11 43/44] tomoyo: recognize kernel threads correctly gregkh
2021-03-08 12:35 ` [PATCH 5.11 44/44] r8169: fix resuming from suspend on RTL8105e if machine runs on battery gregkh
2021-03-08 22:29 ` [PATCH 5.11 00/44] 5.11.5-rc1 review Guenter Roeck
2021-03-09 10:26   ` Greg KH
2021-03-09  4:22 ` Naresh Kamboju
2021-03-09 10:26   ` Greg Kroah-Hartman

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=20210308122719.527723820@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=casey@schaufler-ca.com \
    --cc=dsterba@suse.com \
    --cc=fdmanana@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).