Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Date:   Mon, 27 Nov 2023 14:47:19 -0800
From:   Eric Biggers <ebiggers@kernel.org>
To:     Sergei Shtepa <sergei.shtepa@linux.dev>
Cc:     axboe@kernel.dk, hch@infradead.org, corbet@lwn.net,
        snitzer@kernel.org, mingo@redhat.com, peterz@infradead.org,
        juri.lelli@redhat.com, viro@zeniv.linux.org.uk, brauner@kernel.org,
        linux-block@vger.kernel.org, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        Sergei Shtepa <sergei.shtepa@veeam.com>
Subject: Re: [PATCH v6 11/11] blksnap: prevents using devices with data
 integrity or inline encryption
Message-ID: <20231127224719.GD1463@sol.localdomain>
References: <20231124165933.27580-1-sergei.shtepa@linux.dev>
 <20231124165933.27580-12-sergei.shtepa@linux.dev>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231124165933.27580-12-sergei.shtepa@linux.dev>
Precedence: bulk

On Fri, Nov 24, 2023 at 05:59:33PM +0100, Sergei Shtepa wrote:
> There is an opinion that the use of the blksnap module may violate the
> security of encrypted data. The difference storage file may be located
> on an unreliable disk or even network storage. 

I think this misses the point slightly.  The main problem is that blksnap writes
data in plaintext that is supposed to be encrypted, as indicated by the bio
having an encryption context.  That's just what it does, at least based on the
last patchset; it's not just "an opinion".  See
https://lore.kernel.org/linux-block/20a5802d-424d-588a-c497-1d1236c52880@veeam.com/

> +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> +	if (bio->bi_crypt_context) {
> +		pr_err_once("Hardware inline encryption is not supported\n");
> +		diff_area_set_corrupted(tracker->diff_area, -EPERM);
> +		return false;
> +	}
> +#endif

The error message for ->bi_crypt_context being set should say
"Inline encryption", not "Hardware inline encryption".  The submitter of the bio
may have intended to use blk-crypto-fallback.

Anyway, this patch is better than ignoring the problem.  It's worth noting,
though, that this patch does not prevent blksnap from being set up on a block
device on which blk-crypto-fallback is already being used (or will be used).
When that happens, I/O will suddenly start failing.  For usability reasons,
ideally that would be prevented somehow.

- Eric