From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f48.google.com (mail-wm1-f48.google.com [209.85.128.48]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9123438B7BF for ; Wed, 4 Feb 2026 21:55:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.48 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770242143; cv=none; b=kEt3FD2F91q1tlX9xPiSXx7oVx20BWCCtxEUniWB02XR8vczuf4v+iy4ef+RJQYC0/umU7fZpD7p0ZTB0yvmNrIVBOmY1RL/h4n3e/aJ+WhvfAEQM3Agsd1yBefansu5wSDjNrjpnpeFdctYUKpI9qcNFjaTwsqxfFogZB+7n08= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770242143; c=relaxed/simple; bh=T5JBBCdCz/Gy5zuk1FsSEae54JwI7mUpkeI+tFTjrbc=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=gTSlharNxFJGJ1GY4ikCLlFblLleSt1E3hUB1gf3kArZCS8og874ppIxjnHaWrxol+XAcMGZeSvx1UEcigr+QdE9VXp6ps6l7Zg1fYmF7JWFb6ZUg/uOSbD2/NLEccrCih2TtJHCmWgkJ1QmYguzsDrJycNinSJ/L+7JteGjO8U= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ixPpDOHu; arc=none smtp.client-ip=209.85.128.48 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ixPpDOHu" Received: by mail-wm1-f48.google.com with SMTP id 5b1f17b1804b1-481188b7760so2200955e9.0 for ; Wed, 04 Feb 2026 13:55:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1770242141; x=1770846941; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=+wzXzQA+EkH19tBHUYwMIXENoflLuEWkjrxiDT3pFdo=; b=ixPpDOHuMN4PA3APZ8NSa9Gzhjl86lnpNaZBXLd4dfdH3wcK503gHxEvLvelfAUAyi 4ZPsSZtp5M8TxsfuKFWTllOJjS2VEMaEegXk0aO0Radz28zJ81ynTdUGxl3JCjE1s+3y kEI6qgExlh1uBD/IfEEtkaqVrAgHjzHyck/VxjSe/PdroyXO5C/scBkSWtJEGFF0WL6x S3Adk4JK+Xa+RElVBE1VQXWKaKpk+9hKgki3YKEFT/vSNO5l7LTFzM3HeGr7bFx9e0ck xVRurmJLzQ/ngytI1L6Fwu5uy4A6TIh+ZrMM1brUwAYHXwd9hoJauU7CViBdfzKVdl4V xY3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1770242141; x=1770846941; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=+wzXzQA+EkH19tBHUYwMIXENoflLuEWkjrxiDT3pFdo=; b=mbJdiR1ycj9gf/BHjg+ViszMhAjUFZw+JDBYSTCZ2eOAbogYUdGNQ2/VblstEVO268 Lc6cWFBA3Zir8FCSVNEnZlZRZTRKfYc/4wiEK4fjoQRQX77Sph+hdN9QgDFiE2Y6mLOk fUDjtbzWHn3NOvSQjNO7+791B5/1UUoVfSLjELxnX0HMR0vyAuDobHcukVA0DM6IhnBv qXFYAPotoC81pj6VAkwh+/m28kAAORcjBTjL1iekuLaXXl6z2Wc6L1cNQLmOKY1YpASa iY4Ikl8xCWInGGjHz7IP8cBR5EPuyW7pVH8MB+VQGdFjwJK1AUDbXFokkAYRoWjUMRLN J5QA== X-Forwarded-Encrypted: i=1; AJvYcCWe83+upNnUSPST1TjoT2klXuZwlihkjU5CDlEshcg2Ix2kD1t0FgD8KhkNgLrt4N85SVIP2vzJQpo6qpo=@vger.kernel.org X-Gm-Message-State: AOJu0YxufxU6RXSDD6EtgyayS9xkIa/CXjpgHRtXjGUAj7KbywVs/sSy sUM9KNqcXt1nafA7gcvt8Xn2h6Gi5aJblbsAKGR8FB3OM4qFmCLo65FezmefMA== X-Gm-Gg: AZuq6aIHF4MnODL1m00vQQfINKwcY/OKo/kGM5exWn70Rc3GvTG/S4Rlie58PPpTxIL XWRSJtQwmouymdM//0rk2oIIG4xrG9fs9YYE1ONTlKSq2Z9AhokkkSrv1+MFHEMn6W0Z/jf+BE9 ShZgOAzRZVjPdOsHBT/yvwDOVumV8sNkv+VVZzaZLrQozIygdrKQEphbz57vdHiRpAraAgRJ5Dm s7TFN+d7/3WWCzoMztWvxuP+vDhWL6v8AZgNZOyk8W1UGU7Zkqs5D/5WIHz4725kkLoPUFDbHGa X0rHuz3gPYh9yiqkbTxubTFfHVeRK2bcJpEUGmaAZ1jTipUTe1fwcZdWLQh7+9DH7SHPxm2V4tV 1aVRoWUy5ymrjRzqueeVkywMNKxe+VWyS0smjd/+FRxdVgCIOr/O3eqduj121VDKZLm03F/pIHS +JIPby39JUc69u5LHo47i1DXNNYWvCeTIWCqC21ePw8m1bzWnjKyfk2w== X-Received: by 2002:a05:600c:8b30:b0:47d:264e:b435 with SMTP id 5b1f17b1804b1-4830e96ada6mr63904605e9.22.1770242140830; Wed, 04 Feb 2026 13:55:40 -0800 (PST) Received: from [192.168.1.19] (adsl-84-226-179-133.adslplus.ch. [84.226.179.133]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4830fe86bebsm26196675e9.10.2026.02.04.13.55.39 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 04 Feb 2026 13:55:40 -0800 (PST) Message-ID: Date: Wed, 4 Feb 2026 22:55:39 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1] ext4: fix journal credit check when setting fscrypt context xattr To: Eric Biggers Cc: linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org, Theodore Ts'o , Andreas Dilger , anthonydev@fastmail.com References: <8feeeec8-7330-47ae-9b54-9e789ebdfae5@gmail.com> <20260204205903.GA2197@quark> Content-Language: en-US From: Simon Weber In-Reply-To: <20260204205903.GA2197@quark> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Thank you for your comments, Eric! On 04.02.26 21:59, Eric Biggers wrote: > This patch doesn't actually apply to the stated base-commit, likely > because of corrupted whitespace. Make sure to use 'git send-email' as > described in Documentation/process/submitting-patches.rst. > > The commit message should be broken into paragraphs, and ideally > shortened a bit. The code comment maybe could be shortened as well. Excuse me for the patch formatting! This is my first kernel contribution, so please bear with me. The patch applied correctly on my end when I sent it to myself, but I seem to have mangled it when sending it to the mailing list. I'll make sure to adapt the patch itself, the commit message and code comment in v2. > Since this is a bug fix, please include an appropriate Fixes tag. I'm not sure which commit I should put in the "Fixes:" tag, since the bug arises from the combination of two commits: Firstly, commit 2f8f5e76c7da7871 introduced passing the handle through fs_data, and secondly, commit c1a5d5f6ab21eb7e introduced the check for sufficient credits in ext4_xattr_set_handle. Should I put the chronologically later commit (which would be the latter)? > The specific scenario I'm concerned about is: > > - FS_IOC_SET_ENCRYPTION_POLICY tries to set a directory to encrypted > - A crash occurs (in no-journal mode), leaving the inode having an > encryption xattr on-disk but not the encrypt flag > - e2fsck doesn't correct the inconsistency > - Userspace sees that the directory isn't encrypted yet and retries > FS_IOC_SET_ENCRYPTION_POLICY. Due to XATTR_CREATE, it fails. I think the scenario you describe is somewhat unlikely, but that doesn't mean that we shouldn't be able to deal with it cleanly of course. However the current patch does not have an issue with this scenario, since when ext4_set_context is called through the path from FS_IOC_SET_ENCRYPTION_POLICY, fs_data(=handle) is NULL and therefore my changed line is not executed. The flag would not be set and the ioctl would execute successfully. My commit message was a bit misleading here, making it sound like the ioctl-path actually reaches my suggested change. I think the assumption "fs_data!=NULL implies that encryption xattr MUST NOT be present" would have to be documented clearly to prevent future issues. I see a few alternative possible approaches to ensuring this more cleanly, let me know if you think this is necessary, and if yes, which solution fits the best into the existing philosophy: - Adapt e2fsck to remove encryption xattrs from inodes which do not have the encrypt flag. This might just be a good idea in general. - Adapt fscrypt_get_policy to fix this issue itself on any inodes it is called on, which happens to check before a new context is set in the ioctl-path. I don't like this approach since it would make a getter function have side effects. - We could also change the void *fs_data argument of ext4_set_context from a handle_t to a new struct containing a flag int as well as a handle_t. Then the given flag (if present) could simply be passed down to ext4_xattr_set_handle, or 0 if no fs_data is given. __ext4_new_inode could then pass that flag through the detour through fs/crypto. This would somewhat "self-document" away the assumption (if someone passes the struct with a flag int, they will know not to set XATTR_CREATE if the xattr is possibly already present). Looking forward to your insights! - Simon