All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: "Luís Henriques" <lhenriques@suse.de>
Cc: Xiubo Li <xiubli@redhat.com>, Jeff Layton <jlayton@kernel.org>,
	"Theodore Y. Ts'o" <tytso@mit.edu>,
	Jaegeuk Kim <jaegeuk@kernel.org>,
	Ilya Dryomov <idryomov@gmail.com>,
	linux-fscrypt@vger.kernel.org, ceph-devel@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] fscrypt: new helper function - fscrypt_prepare_atomic_open()
Date: Mon, 13 Mar 2023 11:09:04 -0700	[thread overview]
Message-ID: <ZA9mwPUg7H/fq0L8@sol.localdomain> (raw)
In-Reply-To: <20230313123310.13040-2-lhenriques@suse.de>

On Mon, Mar 13, 2023 at 12:33:09PM +0000, Luís Henriques wrote:
> + * The regular open path will use fscrypt_file_open for that, but in the
> + * atomic open a different approach is required.

This should actually be fscrypt_prepare_lookup, not fscrypt_file_open, right?

> +int fscrypt_prepare_atomic_open(struct inode *dir, struct dentry *dentry)
> +{
> +	int err;
> +
> +	if (!IS_ENCRYPTED(dir))
> +		return 0;
> +
> +	err = fscrypt_get_encryption_info(dir, true);
> +	if (!err && !fscrypt_has_encryption_key(dir)) {
> +		spin_lock(&dentry->d_lock);
> +		dentry->d_flags |= DCACHE_NOKEY_NAME;
> +		spin_unlock(&dentry->d_lock);
> +	}
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(fscrypt_prepare_atomic_open);
[...]
> +static inline int fscrypt_prepare_atomic_open(struct inode *dir,
> +					      struct dentry *dentry)
> +{
> +	return -EOPNOTSUPP;
> +}

This has different behavior on unencrypted directories depending on whether
CONFIG_FS_ENCRYPTION is enabled or not.  That's bad.

In patch 2, the caller you are introducing has already checked IS_ENCRYPTED().

Also, your kerneldoc comment for fscrypt_prepare_atomic_open() says it is for
*encrypted* directories.

So IMO, just remove the IS_ENCRYPTED() check from the CONFIG_FS_ENCRYPTION
version of fscrypt_prepare_atomic_open().

- Eric

  reply	other threads:[~2023-03-13 18:09 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-13 12:33 [PATCH 0/2] ceph: fscrypt: fix atomic open bug for encrypted directories Luís Henriques
2023-03-13 12:33 ` [PATCH 1/2] fscrypt: new helper function - fscrypt_prepare_atomic_open() Luís Henriques
2023-03-13 18:09   ` Eric Biggers [this message]
2023-03-14  0:53     ` Xiubo Li
2023-03-14  2:25       ` Eric Biggers
2023-03-14  4:20         ` Xiubo Li
2023-03-14  9:25           ` Luís Henriques
2023-03-14 10:15     ` Luís Henriques
2023-03-14 17:56       ` Eric Biggers
2023-03-15 11:08         ` Luís Henriques
2023-03-15 17:12           ` Eric Biggers
2023-03-15 17:59             ` Luís Henriques
2023-03-13 12:33 ` [PATCH 2/2] ceph: switch atomic open to use new fscrypt helper Luís Henriques
2023-03-13 18:11   ` Eric Biggers
2023-03-13 18:42     ` Luís Henriques
2023-03-14  0:38       ` Xiubo Li
2023-03-14  9:27         ` Luís Henriques
2023-03-13 17:11 ` [PATCH 0/2] ceph: fscrypt: fix atomic open bug for encrypted directories Jeff Layton

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=ZA9mwPUg7H/fq0L8@sol.localdomain \
    --to=ebiggers@kernel.org \
    --cc=ceph-devel@vger.kernel.org \
    --cc=idryomov@gmail.com \
    --cc=jaegeuk@kernel.org \
    --cc=jlayton@kernel.org \
    --cc=lhenriques@suse.de \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tytso@mit.edu \
    --cc=xiubli@redhat.com \
    /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.