Linux-mtd Archive on lore.kernel.org
 help / color / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: "Theodore Y. Ts'o" <tytso@mit.edu>
Cc: Satya Tangirala <satyat@google.com>,
	linux-api@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net,
	linux-fscrypt@vger.kernel.org, keyrings@vger.kernel.org,
	linux-mtd@lists.infradead.org, linux-crypto@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,
	Paul Crowley <paulcrowley@google.com>
Subject: Re: [PATCH v7 07/16] fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl
Date: Mon, 29 Jul 2019 12:58:28 -0700
Message-ID: <20190729195827.GF169027@gmail.com> (raw)
In-Reply-To: <20190728192417.GG6088@mit.edu>

On Sun, Jul 28, 2019 at 03:24:17PM -0400, Theodore Y. Ts'o wrote:
> > +
> > +/*
> > + * Try to remove an fscrypt master encryption key.  If other users have also
> > + * added the key, we'll remove the current user's usage of the key, then return
> > + * -EUSERS.  Otherwise we'll continue on and try to actually remove the key.
> 
> Nit: this should be moved to patch #11
> 
> Also, perror(EUSERS) will display "Too many users" which is going to
> be confusing.  I understand why you chose this; we would like to
> distinguish between there are still inodes using this key, and there
> are other users using this key.
> 
> Do we really need to return EUSERS in this case?  It's actually not an
> *error* that other users are using the key.  After all, the unlink(2)
> system call doesn't return an advisory error when you delete a file
> which has other hard links.  And an application which does care about
> this detail can always call FS_IOC_ENCRYPTION_KEY_STATUS() and check
> user_count.
> 

Returning 0 when the key wasn't fully removed might also be confusing.  But I
guess you're right that returning an error doesn't match how syscalls usually
work.  It did remove the current user's usage of the key, after all, rather than
completely fail.  And as you point out, if someone cares about other users
having added the key, they can use FS_IOC_GET_ENCRYPTION_KEY_STATUS.

So I guess I'll change it to 0.

Thanks!

- Eric

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply index

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-26 22:41 [PATCH v7 00/16] fscrypt: key management improvements Eric Biggers
2019-07-26 22:41 ` [PATCH v7 01/16] fs, fscrypt: move uapi definitions to new header <linux/fscrypt.h> Eric Biggers
2019-07-28 15:08   ` Theodore Y. Ts'o
2019-07-26 22:41 ` [PATCH v7 02/16] fscrypt: use FSCRYPT_ prefix for uapi constants Eric Biggers
2019-07-26 22:41 ` [PATCH v7 03/16] fscrypt: use FSCRYPT_* definitions, not FS_* Eric Biggers
2019-07-26 22:41 ` [PATCH v7 04/16] fscrypt: add ->ci_inode to fscrypt_info Eric Biggers
2019-07-28 15:09   ` Theodore Y. Ts'o
2019-07-26 22:41 ` [PATCH v7 05/16] fscrypt: refactor v1 policy key setup into keysetup_legacy.c Eric Biggers
2019-07-28 15:40   ` Theodore Y. Ts'o
2019-07-29 19:37     ` Eric Biggers
2019-07-26 22:41 ` [PATCH v7 06/16] fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl Eric Biggers
2019-07-28 18:50   ` Theodore Y. Ts'o
2019-07-29 19:46     ` Eric Biggers
2019-07-29 20:14       ` Theodore Y. Ts'o
2019-07-26 22:41 ` [PATCH v7 07/16] fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl Eric Biggers
2019-07-28 19:24   ` Theodore Y. Ts'o
2019-07-29 19:58     ` Eric Biggers [this message]
2019-07-31 18:38       ` Eric Biggers
2019-07-31 23:38         ` Theodore Y. Ts'o
2019-08-01  1:11           ` [f2fs-dev] " Eric Biggers
2019-08-01  5:31             ` Theodore Y. Ts'o
2019-08-01 18:35               ` Eric Biggers
2019-08-01 18:46                 ` Eric Biggers
2019-08-01 22:04               ` Eric Biggers
2019-08-02  4:38                 ` Eric Biggers
2019-08-12 14:16                   ` Theodore Y. Ts'o
2019-07-26 22:41 ` [PATCH v7 08/16] fscrypt: add FS_IOC_GET_ENCRYPTION_KEY_STATUS ioctl Eric Biggers
2019-07-28 19:30   ` Theodore Y. Ts'o
2019-07-26 22:41 ` [PATCH v7 09/16] fscrypt: add an HKDF-SHA512 implementation Eric Biggers
2019-07-28 19:39   ` Theodore Y. Ts'o
2019-07-29 20:29     ` Eric Biggers
2019-07-29 21:42       ` James Bottomley
2019-07-26 22:41 ` [PATCH v7 10/16] fscrypt: v2 encryption policy support Eric Biggers
2019-07-28 21:17   ` Theodore Y. Ts'o
2019-07-29 20:46     ` Eric Biggers
2019-07-26 22:41 ` [PATCH v7 11/16] fscrypt: allow unprivileged users to add/remove keys for v2 policies Eric Biggers
2019-07-28 21:22   ` Theodore Y. Ts'o
2019-07-26 22:41 ` [PATCH v7 12/16] fscrypt: require that key be added when setting a v2 encryption policy Eric Biggers
2019-07-28 21:24   ` Theodore Y. Ts'o
2019-07-26 22:41 ` [PATCH v7 13/16] ext4: wire up new fscrypt ioctls Eric Biggers
2019-07-28 21:24   ` Theodore Y. Ts'o
2019-07-26 22:41 ` [PATCH v7 14/16] f2fs: " Eric Biggers
2019-07-30  0:36   ` Jaegeuk Kim
2019-08-02  8:10   ` Chao Yu
2019-08-02 17:31     ` Eric Biggers
2019-08-04  9:42       ` [f2fs-dev] " Chao Yu
2019-07-26 22:41 ` [PATCH v7 15/16] ubifs: " Eric Biggers
2019-07-30  0:39   ` Theodore Y. Ts'o
2019-07-26 22:41 ` [PATCH v7 16/16] fscrypt: document the new ioctls and policy version Eric Biggers
2019-07-29  2:00   ` Theodore Y. Ts'o
2019-07-29 21:36     ` Eric Biggers

Reply instructions:

You may reply publically 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=20190729195827.GF169027@gmail.com \
    --to=ebiggers@kernel.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=paulcrowley@google.com \
    --cc=satyat@google.com \
    --cc=tytso@mit.edu \
    /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

Linux-mtd Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mtd/0 linux-mtd/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mtd linux-mtd/ https://lore.kernel.org/linux-mtd \
		linux-mtd@lists.infradead.org linux-mtd@archiver.kernel.org
	public-inbox-index linux-mtd

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-mtd


AGPL code for this site: git clone https://public-inbox.org/ public-inbox